From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pj1-x102f.google.com (mail-pj1-x102f.google.com [IPv6:2607:f8b0:4864:20::102f]) by sourceware.org (Postfix) with ESMTPS id 7516F3858D39 for ; Mon, 23 Oct 2023 22:03:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 7516F3858D39 Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=chromium.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=chromium.org ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 7516F3858D39 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::102f ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698098607; cv=none; b=Ik88gg8Rdp+5F8w1OSW/q8T0NhOWuZQ05kCMNdoy2kK8/54ak6hrIfxHOtGCLlsvRywaW8EWBno7DinBjDS0SsmkTs3EVjvt9LPERUnNHAi5ilTgWl2WzEJLVnuaIY0XH3swyXfyL+/4qyK/Rk8SYxqQsdQc+ZiuTbyOse2N6yk= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698098607; c=relaxed/simple; bh=ymDMRGG5PNJzd56c1nNaYiMTqLKJ4WssbemrmmbSGlc=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=KEiHDOtN/byhg6F4G+HarhAMDaM9s0w5bUlsPzqNtQWECD5VaZ8NqpA6zPQfaCngEMbhE7qZPpVeJK5PTm4X0kIa9TgodA3uQU23yagcphOYUcVlywYWdfIpI7GEUNcWoNXA7S49iP2IEsA6VitWnLMbFzBzdNNbndFOCwZiJTc= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pj1-x102f.google.com with SMTP id 98e67ed59e1d1-27d1aee5aa1so2766276a91.0 for ; Mon, 23 Oct 2023 15:03:25 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1698098604; x=1698703404; darn=gcc.gnu.org; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date:from:to :cc:subject:date:message-id:reply-to; bh=tlM4ejXJ4SfjVsOM5X63LGFZGTHtoqERvXqXYg9Y+Xo=; b=BRaHVAGfnLc8Rcx81qyRvH51bljOARdtBtevZd/eyax478r3pmxi6oPPZftol52x7q Xriz6Oew2a/PANeNvYmJ4YEXw8RuU4/pvzHbw07sbZw88Eorcwd06yimSFWa1m25+LWj /2baON1UFKRaIOw8Mr1h7bkUqxHOurMDftClc= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698098604; x=1698703404; h=in-reply-to:content-transfer-encoding:content-disposition :mime-version:references:message-id:subject:cc:to:from:date :x-gm-message-state:from:to:cc:subject:date:message-id:reply-to; bh=tlM4ejXJ4SfjVsOM5X63LGFZGTHtoqERvXqXYg9Y+Xo=; b=p2bqm2Tlo/7reMAMDYgmjdU0kiVb9j0g4781c3MLlvRjg+82cqsLWnvAVryd8rlyab wg5u6V3I0CGorWW0HJ5I+lA2v+TtexbmcN0QijHOEV43bqxbEEN4HlZBWnhR8k3w2P+W ArjQwBof93+wOPnh38EWWnm7+cuiHiIFWT5Q3NOblv9gXCqaIZRlH1j9bE7qBxsGpG5N wotmNquAw15VZ8aUEdDaF8y5PyRhQJ+RiZOJZetyWJHtMQgY5ZPzX80UY5kE0VfQzSyy mRWex+9thL7eW/Qx+B5HnD0Ibr5vA5C2l8rzA03OLI5+HwdfhJB5Crvttk+b8JQ94tr9 a+7w== X-Gm-Message-State: AOJu0YxcRpuWEMnrm6RKOINT4dJO/p0AB3XdUPv1W1A8fgUq/cbp5S9k lWX4WYhWtp33hJKHJ+d5wPn1YeoOk6nxevHWkis= X-Google-Smtp-Source: AGHT+IGUtID73M+4hGcd36MsbyIWDBGXtd8CSBekGyaylYD/lv2SY8Dcf511StTbfGn+E4KbacakGA== X-Received: by 2002:a17:90a:c20d:b0:277:61d7:78be with SMTP id e13-20020a17090ac20d00b0027761d778bemr17816651pjt.14.1698098604054; Mon, 23 Oct 2023 15:03:24 -0700 (PDT) Received: from www.outflux.net (198-0-35-241-static.hfc.comcastbusiness.net. [198.0.35.241]) by smtp.gmail.com with ESMTPSA id n32-20020a17090a2ca300b0027d015c365csm8735852pjd.31.2023.10.23.15.03.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Oct 2023 15:03:23 -0700 (PDT) Date: Mon, 23 Oct 2023 15:03:23 -0700 From: Kees Cook To: Martin Uecker Cc: Qing Zhao , gcc-patches@gcc.gnu.org Subject: Re: [V3][PATCH 0/3] New attribute "counted_by" to annotate bounds for C99 FAM(PR108896) Message-ID: <202310231501.3FCE176C@keescook> References: <20230825152425.2417656-1-qing.zhao@oracle.com> <202310191631.C57B952@keescook> <56fae6f212cab1130cd27cee46a19446e9d92dab.camel@tugraz.at> <202310201132.9D8E5CAC@keescook> <0CBEB5DC-B05F-4934-A1C7-D7364CCD7F93@oracle.com> <84d75495ef3704b05b62a6adc30e64e49c135f0b.camel@tugraz.at> <202310231250.EF0C7A3A@keescook> <2cb8c622ec414303679f73bedee7aa083760f460.camel@tugraz.at> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <2cb8c622ec414303679f73bedee7aa083760f460.camel@tugraz.at> X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,JMQ_SPF_NEUTRAL,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP 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: On Mon, Oct 23, 2023 at 09:57:45PM +0200, Martin Uecker wrote: > Am Montag, dem 23.10.2023 um 12:52 -0700 schrieb Kees Cook: > > On Fri, Oct 20, 2023 at 09:54:05PM +0200, Martin Uecker wrote: > > > Am Freitag, dem 20.10.2023 um 18:48 +0000 schrieb Qing Zhao: > > > > > > > > > On Oct 20, 2023, at 2:34 PM, Kees Cook wrote: > > > > > > > > > > On Fri, Oct 20, 2023 at 11:50:11AM +0200, Martin Uecker wrote: > > > > > > Am Donnerstag, dem 19.10.2023 um 16:33 -0700 schrieb Kees Cook: > > > > > > > On Wed, Oct 18, 2023 at 09:11:43PM +0000, Qing Zhao wrote: > > > > > > > > As I replied to Martin in another email, I plan to do the following to resolve this issue: > > > > > > > > > > > > > > > > 1. No specification for signed or unsigned for counted_by field. > > > > > > > > 2. Add a sanitizer option -fsanitize=counted-by-bound to catch the cases when the size of the counted-by is not positive. > > > > > > > > > > > > > > I don't understand why this needs to be a runtime sanitizer. The > > > > > > > signedness is known at compile time, so I would expect a -W option. > > > > > > > > > > > > The signedness of the type but not of the value. > > > > > > > > > > > > But I would not want to have a warning for signed > > > > > > counter types by default because I would prefer > > > > > > to use signed types (for various reasons including > > > > > > better overflow detection). > > > > > > > > > > > > > Or > > > > > > > do you mean you'd split up -fsanitize=bounds between unsigned and signed > > > > > > > indexes? I'd find that kind of awkward for the kernel... but I feel like > > > > > > > I've misunderstood something. :) > > > > > > > > > > > > > > -Kees > > > > > > > > > > > > The idea would be to detect at run-time the case > > > > > > if x->buf is used at a time where x->counter > > > > > > is negative and also when x->counter * sizeof(x->buf[0]) > > > > > > overflows or is too big. > > > > > > > > > > > > This would be similar to > > > > > > > > > > > > int a[n]; > > > > > > > > > > > > where it is detected at run-time if n is not-positive. > > > > > > > > > > Right. I guess what I mean to say is that I would expect this case to > > > > > already be caught by -fsanitize=bounds -- I don't see a reason to add an > > > > > additional sanitizer option. > > > > > > > > > > struct foo { > > > > > int count; > > > > > int array[] __counted_by(count); > > > > > }; > > > > > > > > > > foo->count = 5; > > > > > foo->array[0] = 1; // ok > > > > > foo->array[10] = 1; // -fsanitize=bounds will catch this > > > > > foo->array[-10] = 1; // -fsanitize=bounds will catch this too > > > > > > > > > > > > > > > > > > just checked this testing case with my GCC, and YES, -fsanitize=bounds indeed caught this error: > > > > > > > > ttt_1.c:31:12: runtime error: index 10 out of bounds for type 'char [*]' > > > > ttt_1.c:32:12: runtime error: index -10 out of bounds for type 'char [*]’ > > > > > > > > > > Yes, but I thought we were discussing the case where count is > > > set to a negative value: > > > > > > foo->count = -1; > > > int x = foo->array[3]; // UBSan should diagnose this > > > > Oh right, I keep thinking about it backwards. > > > > Yeah, we can't trap the "count" assignment, because it may be getting used > > for other purposes. But yeah, access to "array" should trap if "count" > > is negative. > > > > > And also the case when foo->array becomes too big. > > > > How do you mean? > > count * sizeof(member) could overflow or otherwise be > bigger than allowed. Ah! Yes. foo->count = SIZE_MAX; foo->array[0]; // UBSan diagnose: // SIZE_MAX * sizeof(int) is larger than can be represented > > Martin > > -- Kees Cook