From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mailrelay.tugraz.at (mailrelay.tugraz.at [129.27.2.202]) by sourceware.org (Postfix) with ESMTPS id BBF763858023 for ; Wed, 19 Jul 2023 08:41:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org BBF763858023 Authentication-Results: sourceware.org; dmarc=pass (p=quarantine dis=none) header.from=tugraz.at Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=tugraz.at Received: from vra-170-232.tugraz.at (vra-170-232.tugraz.at [129.27.170.232]) by mailrelay.tugraz.at (Postfix) with ESMTPSA id 4R5Tnl22zPz3wVL; Wed, 19 Jul 2023 10:41:46 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=tugraz.at; s=mailrelay; t=1689756109; bh=BzIDOVDd41e7WtR9YkMM6ZYkFkVbMtreb99FV2wACcA=; h=Subject:From:To:Cc:Date:In-Reply-To:References; b=HsanyzzysWLvNNmO/RTrVQY7qDYY9U8jeEiF8ZBfOAfWNX3W1lSPM4E3zN5EVnx55 flHtHh0gQV4slOy2OmjnpKTXhJv+YYb4ajgYa2YwS3bc7PUEjp7uVxrV5cZhj2EmIm py1Tee1cH0jmkIclZAheVCllm4jFM3IRK8pNR4pM= Message-ID: Subject: Re: [V1][PATCH 0/3] New attribute "element_count" to annotate bounds for C99 FAM(PR108896) From: Martin Uecker To: Kees Cook , Qing Zhao Cc: "joseph@codesourcery.com" , "richard.guenther@gmail.com" , "jakub@redhat.com" , "gcc-patches@gcc.gnu.org" , "siddhesh@gotplt.org" , "isanbard@gmail.com" Date: Wed, 19 Jul 2023 10:41:44 +0200 In-Reply-To: <202307171612.406D82C39@keescook> References: <20230525161450.3704901-1-qing.zhao@oracle.com> <202305261218.2420AB8E0@keescook> <202307131311.1F30C4357@keescook> <202307171612.406D82C39@keescook> Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable User-Agent: Evolution 3.46.4-2 MIME-Version: 1.0 X-TUG-Backscatter-control: G/VXY7/6zeyuAY/PU2/0qw X-Spam-Scanner: SpamAssassin 3.003001 X-Spam-Score-relay: -0.4 X-Scanned-By: MIMEDefang 2.74 on 129.27.10.116 X-Spam-Status: No, score=-4.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,KAM_SHORT,RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,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: Am Montag, dem 17.07.2023 um 16:40 -0700 schrieb Kees Cook: > On Mon, Jul 17, 2023 at 09:17:48PM +0000, Qing Zhao wrote: > >=20 > > > On Jul 13, 2023, at 4:31 PM, Kees Cook > > > wrote: > > >=20 > > > In the bug, the problem is that "p" isn't known to be allocated, > > > if I'm > > > reading that correctly? > >=20 > > I think that the major point in PR109557 > > (https://gcc.gnu.org/bugzilla/show_bug.cgi?id=3D109557): > >=20 > > for the following pointer p.3_1,=20 > >=20 > > p.3_1 =3D p; > > _2 =3D __builtin_object_size (p.3_1, 0); > >=20 > > Question: why the size of p.3_1 cannot use the TYPE_SIZE of the > > pointee of p when the TYPE_SIZE can be determined at compile time? > >=20 > > Answer:=C2=A0 From just knowing the type of the pointee of p, the > > compiler cannot determine the size of the object.=C2=A0=20 >=20 > Why is that? "p" points to "struct P", which has a fixed size. There > must be an assumption somewhere that a pointer is allocated, > otherwise > __bos would almost never work? It often does not work, because it relies on the optimizer propagating the information instead of the type system. This is why it would be better to have proper *bounds* checks, and not just object size checks. It is not quite clear to me how BOS and bounds checking is supposed to work together, but FAMs should be bounds checked.=20 ... >=20 > >=20 > > > This may be > > > desirable in a few situations. One example would be a large > > > allocation > > > that is slowly filled up by the program. > >=20 > > So, for such situation, whenever the allocation is filled up, the > > field that hold the =E2=80=9Ccounted_by=E2=80=9D attribute should be in= creased at > > the same time, > > Then, the =E2=80=9Ccounted_by=E2=80=9D value always sync with the real = allocation.=20 > > > I.e. the counted_by member is > > > slowly increased during runtime (but not beyond the true > > > allocation size). > >=20 > > Then there should be source code to increase the =E2=80=9Ccounted_by=E2= =80=9D field > > whenever the allocated space increased too.=20 > > >=20 > > > Of course allocation size is only available in limited > > > situations, so > > > the loss of that info is fine: we have counted_by for everything > > > else. > >=20 > > The point is: allocation size should synced with the value of > > =E2=80=9Ccounted_by=E2=80=9D. LLVM=E2=80=99s RFC also have the similar = requirement: > > https://discourse.llvm.org/t/rfc-enforcing-bounds-safety-in-c-fbounds-s= afety/70854#maintaining-correctness-of-bounds-annotations-18 >=20 > Right, I'm saying it would be nice if __alloc_size was checked as > well, > in the sense that if it is available, it knows without question what > the > size of the allocation is. If __alloc_size and __counted_by conflict, > the smaller of the two should be the truth. >=20 > But, as I said, if there is some need to explicitly ignore > __alloc_size > when __counted_by is present, I can live with it; we just need to > document it. >=20 > If the RFC and you agree that the __counted_by variable can only ever > be > (re)assigned after the flex array has been (re)allocated, then I > guess > we'll see how it goes. :) I think most places in the kernel using > __counted_by will be fine, but I suspect we may have cases where we > need > to update it like in the loop I described above. If that's true, we > can > revisit the requirement then. :) It should be the other way round: You should first set 'count' and then reassign the pointer, because you can then often check the pointer assignment (reading 'count'). The other way round this works only sometimes, i.e. if both assignments are close together and the optimizer can see this. Martin