From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x332.google.com (mail-ot1-x332.google.com [IPv6:2607:f8b0:4864:20::332]) by sourceware.org (Postfix) with ESMTPS id EF709385800A for ; Wed, 24 May 2023 12:04:39 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EF709385800A Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=linaro.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=linaro.org Received: by mail-ot1-x332.google.com with SMTP id 46e09a7af769-6a984993740so65765a34.2 for ; Wed, 24 May 2023 05:04:39 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; t=1684929879; x=1687521879; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:from:to:cc:subject:date:message-id:reply-to; bh=n9os3waCErHZkIusydXLm6rTMplZZvhPBur28O3T1nU=; b=Wgs2wW8/BH6FOb3soJTP6Ck8jl6PLNNIhDyqRukyDXtXILZzy7J/sbYu7YrGe8yEou 155gQi2PzzlBxWV9V7fQbKIG2R9nIeauvffo4EStQpcchM0BDz7mkbjf+OnTpLA0XBBe iBKZv7waFaCqSwbIPbidpSF2ahT018Wwvrobbw/2roT30qtXLt4UwMH2q7VXj5iJiz8H Fy39YOGrZs0u9Cjn8L4GAaFnKuh9i6Nc3fvjZ2MbfWjq/ij+n7biKRAzqyPFsJYGrUml 6YAXeB5Rihtkz8WB/5U8Q4/KKybE+xcwkaFkTh6gFuDaDBjqU6BS+6Q01Q41t7WUVLo6 91iw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684929879; x=1687521879; h=content-transfer-encoding:in-reply-to:organization:from:references :cc:to:content-language:subject:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=n9os3waCErHZkIusydXLm6rTMplZZvhPBur28O3T1nU=; b=jrWUWKlmsFEwCPfevuATgHBUAwNwTSmCjwjVO6eVQGIxfgCiyhgKfK601ZjwQnUmCt 4KoJhxON97zbbIrspY73expy94clPiK0TuLMezPHGSIgFiZSS2kgvjeFsGFAk+1hO42t cAlouUnnjp6LebqTM4vKE/8MoX5bmT7CLQn3nfdHKi7yiLakJZFwDha57ghBAWZ5OEe4 614E111Cqu3VxH6Va7964jkT9RDi/NyicksP7dGIqXyrBRloqEnIDvqSA+YYkTpVxNN5 knZw7bJqWX7px2hRzKsegNeLhXOTD5HEgdRFMusD/x+TfpKULhNkDo6qj+6CidY6T30X Bm3A== X-Gm-Message-State: AC+VfDz5AZMsCyg+irZN6QHD4JDHQzDwxg3TOTjjFg6RBgpP9ZplJD2B mvvt493zTszl6TOswllp9JFjgzyUR3rletd0fDS+IQ== X-Google-Smtp-Source: ACHHUZ4SGyl446XdTuptMbLihRa9pxP/eGR6e4NN5OWmTrYMUfKT7bii+oLM/5P8v4ajBYMb/ln3Vg== X-Received: by 2002:a05:6830:1e75:b0:6af:9b72:979f with SMTP id m21-20020a0568301e7500b006af9b72979fmr1011135otr.24.1684929879189; Wed, 24 May 2023 05:04:39 -0700 (PDT) Received: from ?IPV6:2804:1b3:a7c1:2e05:a4ad:a873:8177:a796? ([2804:1b3:a7c1:2e05:a4ad:a873:8177:a796]) by smtp.gmail.com with ESMTPSA id u21-20020a056830119500b006af800065f2sm2907021otq.59.2023.05.24.05.04.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Wed, 24 May 2023 05:04:38 -0700 (PDT) Message-ID: <6bbaa591-2a8f-6b1d-c649-98dbaf2e9fce@linaro.org> Date: Wed, 24 May 2023 09:04:36 -0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [RFC PATCH 1/1] io: Add FORTIFY_SOURCE check for fcntl arguments Content-Language: en-US To: Sergey Bugaev Cc: libc-alpha@sourceware.org References: <20230519213059.3812385-1-bugaevc@gmail.com> <20230519213059.3812385-2-bugaevc@gmail.com> <0b512019-09ab-9f25-57d6-cf6fd19be8f3@linaro.org> From: Adhemerval Zanella Netto Organization: Linaro In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-6.3 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On 23/05/23 17:24, Sergey Bugaev wrote: > On Tue, May 23, 2023 at 10:57 PM Adhemerval Zanella Netto > wrote: >>>>> + if (__fcntl_cmd_needs_arg (__cmd) && __va_arg_pack_len () < 1) >>>> >>>> No implicit check for function that do not return bool: >>>> >>>> if (__fcntl_cmd_needs_arg (__cmd) == 1 ...) >>> >>> Why? Is it just a code style thing? >> >> Yes, it is from the glibc code style (check 'Boolean Coercions' [1]). >> >> [1] https://sourceware.org/glibc/wiki/Style_and_Conventions > > [I don't mean to argue, and surely you know better, and not that I > care about this], but I read that as saying that implicit > integer-to-bool conversions are frowned upon, not ints used as bools > (because C used to lack a bool type). In other words: if you have an > integer variable that can have many different values and you want to > check it against 0, you're supposed to write out the != 0 explicitly. > But if what you have is essentially a boolean, but you use int/1/0 > instead of bool/true/false, then just checking if (my_bool) should be > fine, and doing != 0 would be awkward -- no? > > I've surely seen glibc code use if (my_bool) without the != 0 when > my_bool is declared as an int. __libc_enable_secure is one example. The interger as bool is not the problem, specially on a installed header that won't be easy to use 'bool' (due namespace pollution, standard conformance, etc.). And sure some usage of implicit conversions might have slip in review, but I think it should follow the code guide lines anyway (unless we revise it). > >>> The idea here was that the 2-argument version of fcntl clearly does >>> not deal with time, either 32- or 64-bit... >> >> And that's why we don't have a __fcntl_time64 implementation. But Florian >> has asked to add one anyway if we even eventually need to handle some time >> related structure. >> >> I think we can skip this for now, but at least add a comment stating that >> if we even need to handle time related field with fcntl we will a new >> redirection. > > Hm, either I'm failing to understand what you're saying (which is > possible: it is late at night), or maybe I didn't make my point clear: > > if what you're concerned is what the _2 functions end up calling > (__fcntl_time64 vs __libc_fcntl64), then it does not matter because > they're only used for 2-argument variant of fcntl, which can not > possibly deal with any time-related structures (nor any structures), > exactly because there's no third argument where a structure could be > passed. The 2-arg variant is basically for simple getters that return > an int. > > But if something is off (wrt time64 vs not) in the main fortification > / redirection code, then I surely need to fix that -- then please > clarify what specific configuration / case you're talking about. Right, I see your point now. > Ah, no, I see what you mean: you mean we don't strictly speaking need > the __fcntl64_2 call immediately following __fcntl_missing_arg. > > I guess we don't -- maybe the corresponding change needs to be done to > the open* fortification as well then. Agree.