From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x42a.google.com (mail-pf1-x42a.google.com [IPv6:2607:f8b0:4864:20::42a]) by sourceware.org (Postfix) with ESMTPS id EC6013858D37 for ; Mon, 23 Oct 2023 19:52:18 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org EC6013858D37 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 EC6013858D37 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::42a ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698090740; cv=none; b=LmFicoxEY0fzZXE84hk6/+AhJP/AWYQqqLsS0oNqzfDxg2VjI7FydkjwkgWc7LQWXCZjEtpz0+snV11T6lW+JKCxhH6pDBrC+sSjJwSYCEA1n6sTRDY3Cl2eZ7O0sz446m8c9TexTCU4Bftrzyr1ZjoRGATDoIN1zfnMR/zvxBw= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1698090740; c=relaxed/simple; bh=hJ/eZNcT9TmuHWV9wnU3ctiTrtCQzRim14l8DA98X8c=; h=DKIM-Signature:Date:From:To:Subject:Message-ID:MIME-Version; b=t4h5gZCSpo2W3NupMIBaNcaitaIOdpj2wTf4yHHv2C67719CMwOcMxsW+r1JQjuFDjckZT6AGortZC38tIyWH6nbNkJGiMSZJKQZI12Ln0/MvTEc6MISBPNeYsCMqKhMDLscq73lyJMwFuDH2BTrR/QuQBfo8Otx+BUKBcFFfw8= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pf1-x42a.google.com with SMTP id d2e1a72fcca58-6b87c1edfd5so2827072b3a.1 for ; Mon, 23 Oct 2023 12:52:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=chromium.org; s=google; t=1698090737; x=1698695537; 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=I7BkAG5hpUay3qbmc9UdbUb2PrZ74PHVlDX0Qz8bvbk=; b=ItwSHsNRE09jZ8rmg93+QHcZcRpo7XNgrHl1ErOgFO/hAHEj7aoMtaRGFYvDt/n4zj S7QJoFwBMhSXicPH0rVICYF0x9Bl5ZHuc99GkJLFq2Q5eAQuKQjC3N75wl80dOEAi74Y NL+o3REN4WvUAEoJ711/TtpTC0ITbcXggtv7k= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1698090737; x=1698695537; 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=I7BkAG5hpUay3qbmc9UdbUb2PrZ74PHVlDX0Qz8bvbk=; b=hDJMvG+aD/qAoT0L4X7JK8N3TUDvw+x+HBFt3WH3o8OpeWgkqUFA4V+ylznO4PqA6G M/rZLtxylhFaEvkCv1AyHrbITr4aRJvec9D9EpeBJhbtg0B44/t7YSWrkL3GMRdHuiHY hoo8+eHno3+YBPs9gf162KC7MQki0A4jMW7PNHNBitH5Fq6XEjTPojrhAlfpJHnCvYrv EpXchxlgXHH5MxfJ5y5QCjDScpIZwAvdF5AvfgX8GPzfO4K7V1crqahZ4elCc11SKU3U XiJZR9Lz1b77y7KhwJWaXtPFr6Q+SiZYFEOame5MbzSWQoXvccqGmuJ4ZRt0frpmbqiO xsyA== X-Gm-Message-State: AOJu0YwGPjRmzMxgwfDm8IyhuZIq5KzEX+gpNQqrTLaAulYRquDty0MT ibDmAtWHGtEOc1SfwiAPyFvfnKu7vi06rG1RgNk= X-Google-Smtp-Source: AGHT+IHbX3JetlyV6dXYpU/Ut5pzbMrXXlAYt3EjFuSqwOzytV2tC5TFjFXCK5AbhqymcW6fxlohGA== X-Received: by 2002:a62:7948:0:b0:690:d620:7804 with SMTP id u69-20020a627948000000b00690d6207804mr7229890pfc.13.1698090737371; Mon, 23 Oct 2023 12:52:17 -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 t22-20020aa79476000000b0068e49cb1692sm6448254pfq.1.2023.10.23.12.52.16 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 23 Oct 2023 12:52:16 -0700 (PDT) Date: Mon, 23 Oct 2023 12:52:16 -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: <202310231250.EF0C7A3A@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> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <84d75495ef3704b05b62a6adc30e64e49c135f0b.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 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? -- Kees Cook