From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-lf1-x12a.google.com (mail-lf1-x12a.google.com [IPv6:2a00:1450:4864:20::12a]) by sourceware.org (Postfix) with ESMTPS id B79CA3858D38 for ; Sun, 28 May 2023 17:20:17 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org B79CA3858D38 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-lf1-x12a.google.com with SMTP id 2adb3069b0e04-4f13d8f74abso2698773e87.0 for ; Sun, 28 May 2023 10:20:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1685294415; x=1687886415; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:from:to:cc:subject:date:message-id:reply-to; bh=KLV2qHskLW5VFLp5YSD2qbeATdMCMFAV6uEAPNMOu14=; b=QjwGfdNwoAXFV0vdhHEKdQhF1GL+xOBCgfhc8DV5+5sEwWVu34zY087n5J7KwwvbkX xiXqXWLnIXr2iQedfjiHLz/IXQkVRdwSBePMmdXg6Tp7P9u7Bhe2KOCWubN+Aq2KGEUg Clw6trYGca1zwQM7R2r1+rDEzUpIRpZ4N5H7vV7U3orVa4G9yp/yFonGGWofBg5arr94 l5oR6Om1G4cOLXiq1+L5rvGBdMk4s43QT0uFeY9xaCk8lYEK2tzylYAtuJaz/LubGfgh EK27UfNLxmRkC8y67UEBNMC+OHyjYTfCh3AhcrBoqobKoBmY+ZvQlEDkNVYujj0CryeX XIog== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1685294415; x=1687886415; h=content-transfer-encoding:mime-version:message-id:date:subject:to :from:x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=KLV2qHskLW5VFLp5YSD2qbeATdMCMFAV6uEAPNMOu14=; b=MfwFd7TZv3LKWKsteOCcwmZ6/vbqXraDt341EdV1zxRDodMj0ubwM1ZSyj7LCocRiR 1aU8uw0kTtbLqnQSu3xt9VYrl36r21RW6ToNX2yR1ceZ65nfWV7pdhes2K3k66L8WejK G/+s/KT4bZx+0kb+wINnGbebJiIlmk/h/wxF+jCbLUdJaLJLd28cypJPm7YEGQPGaHHc bkCCVwpqQHk81T9bRIfYh5AfRkjO3SuaLNFnr5dSQIef0vfldEKdTSspzfJieT11vEl+ d4jgG8drgN0RykBOA5Lr0j7brXLmqVjHxbmgg3i8XeX25cAUx9/EwbEFanfWyK30Drjv ri+g== X-Gm-Message-State: AC+VfDwmjI5dsUJq4rGvIij50WyeCndNdZgmKjxsr3+MtfOWom98xW6V MAO36rQ/pLBp6h/Z+Bz0TQ9SG3sFs+4= X-Google-Smtp-Source: ACHHUZ5TnU0IvvBwZ6mN0RxfW15SspzR9xjzSN6T2j+PAr9QZYajguAn8Wbhp+A1ymcxQzxqlHoIPQ== X-Received: by 2002:ac2:5e91:0:b0:4f4:c909:cdda with SMTP id b17-20020ac25e91000000b004f4c909cddamr2758562lfq.46.1685294415266; Sun, 28 May 2023 10:20:15 -0700 (PDT) Received: from surface-pro-6.. ([2a00:1370:818c:4a57:5e3d:a067:bc18:2967]) by smtp.gmail.com with ESMTPSA id v26-20020a19741a000000b004f4d5003e8dsm1662596lfe.7.2023.05.28.10.20.14 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Sun, 28 May 2023 10:20:14 -0700 (PDT) From: Sergey Bugaev To: libc-alpha@sourceware.org Subject: [PATCH v2 0/3] fcntl fortification Date: Sun, 28 May 2023 20:20:10 +0300 Message-Id: <20230528172013.73111-1-bugaevc@gmail.com> X-Mailer: git-send-email 2.40.1 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-3.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,KAM_ASCII_DIVIDERS,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=no autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: Hello, this is the v2. Changes compared to v1: - The check for whether the kernel supports OFD locks is now in a separate libsupport function, support_fcntl_support_ofd_locks (). - __fcntl64_2 () is gone, a single __fcntl_2 () is used for all the cases. This is because the 2-argument version doesn't deal with file sizes or times, because of there not being a third argument. - Addressed misc review comments. - The big one: it now does argument type checking! So, let's talk about type checking for a bit. It is *not* implemented with _Generic (or other type-based dispatch, such as C++ function overloading). The crucial point here is not failing on custom/unknown fcntl commands; for such commands we don't want to emit any errors (either compile-time or runtime) if used in the 2-argument form, and don't want to emit any type mismatch warnings either. So if we went off the type like I was initially planning to, we'd have to list all known commands *incompatible* with the type, and that would be unmaintainable. So the way this is done is: 1. There are __fcntl_is_xxxx (cmd) inline functions, for example __fcntl_is_int (), that only return 1 if the command is known to require an argument of the given type. For flock types, there's a second argument, __is_fcntl64 -- since the OFD lock commands expect a struct flock64 when used in fcntl64, and a plain struct flock in plain fcntl -- unlike the non-OFD locks, where e.g. F_SETLK always references a struct flock, and F_SETLK64 always a struct flock64. Please correct me on this if my understanding is wrong!! 2. There is a __fcntl_types_compatible () macro which is a thin wrapper over __builtin_types_compatible_p () in plain C, and uses an std::is_same_v-like check (using partial template specialization) in C++. Importantly, it uses __typeof () even in C++ (not decltype ()), because we don't want the extra references appended to our type. For example, we want 'int', not 'const int &' or 'int &&'. 3. There are __fcntl_type_check_xxxx (arg) macros that check that the type of arg is compatible with the expected type. For simple cases this is only a matter of using __fcntl_types_compatible (arg, xxxx), and for those cases these macros only serve to reduce the boilerplate a bit. However for the const pointers we also have to check for the non-const type version (since __builtin_types_compatible_p () does not do this on its own). No attempt is made to check for volatile. 4. There's the __fcntl_type_check (cmd, arg, is_fcntl64) macro that evaluates to 0 or 1 depending on whether the types match. It consists of a chain of checks like this one: __fcntl_is_int (cmd) ? __fcntl_type_check_int (arg) : and terminates with a 1, so any unrecognized command always passes the type check. To avoid tons and tons of nested parenthesis, the implementation replies on the precedence of the ternary operator in C: specifically, it works just like an if / else if / else chain, so you can have __fcntl_is_foo (cmd) ? __fcntl_type_check_foo (arg) : __fcntl_is_bar (cmd) ? __fcntl_type_check_bar (arg) : and so on. I would not normally do this (better add explicit parens to make it clearer to the humans reading the code), but in this case it is more clear than adding many, many parens. 5. Here's the fcntl () macro in all of its horrible glory: #define fcntl(fd, cmd, ...) (__VA_OPT__ (0 ?) __fcntl_2_inline (fd, cmd) __VA_OPT__ (: !__builtin_constant_p (cmd) ? __fcntl_alias (fd, cmd, __VA_ARGS__) : __fcntl_type_check (cmd, __VA_ARGS__, 0) ? __fcntl_alias (fd, cmd, __VA_ARGS__) : __fcntl_warn (fd, cmd, __VA_ARGS__))) First, the __VA_OPT__ () trick: if there only are two arguments, this expands to __fcntl_2_inline (fd, cmd), and that's it. Otherwise it expands to 0 ? __fcntl_2_inline (fd, cmd) : (the rest), and (the rest) is where the 3-argument logic is implemented. If cmd is not a compile-time const, it just forwards to __fcntl_alias (), otherwise it does the __fcntl_type_check (), and forwards to __fcntl_alias () or __fcntl_warn () depending on the result. 6. __fcntl_warn () is basically the same as __fcntl_alias (), except it's defined with __warnattr. So you get a warning (not a hard error) on type mismatch. This is in line with how pointer type mismatch is handled elsewhere in C / GCC. You can of course escalate this to an error with -Werror if you want to. Forgeting an argument when it's required is still a hard error (__errordecl). To make the warning (from __warnattr) to show up when building against fcntl2.h as a "system header", I had to wrap it in a "pragma gcc diagnostic"; please see the patch for a longer explanation. Here's what it looks like in practice: Forgetting a required argument: ------------------------------------------------------------------ In file included from /usr/include/fcntl.h:342, from demo.c:3: In function ‘__fcntl_2_inline’, inlined from ‘main’ at demo.c:6:10: /usr/include/bits/fcntl2.h:541:5: error: call to ‘__fcntl_missing_arg’ declared with attribute error: fcntl with with this command needs 3 arguments 541 | __fcntl_missing_arg (); | ^~~~~~~~~~~~~~~~~~~~~~ ------------------------------------------------------------------ Type mismatch: ------------------------------------------------------------------ In file included from /usr/include/fcntl.h:342, from demo.c:3: demo.c: In function ‘main’: demo.c:7:10: warning: call to ‘__fcntl_warn’ declared with attribute warning: fcntl argument has wrong type for this command [-Wattribute-warning] 7 | return fcntl (0, F_DUPFD, ptr); | ^~~~~ ------------------------------------------------------------------ I have made no attempt to guard all of this with a compiler version prereq! This seems appropriate because it's not using _Generic, but maybe it's still worth doing. If it is, please tell me what the correct check would be and I'll put it in. I have only really tested with (modern) GCC. I briefly checked with Clang, but the fortification doesn't seem to get enabled at all; perhaps it's failing some other check. Sergey