From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f48.google.com (mail-wm1-f48.google.com [209.85.128.48]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9FD4131F992 for ; Wed, 1 Apr 2026 18:29:36 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775068178; cv=none; b=ZlS8s/QOyW3ORBM9jbyEBuNsIJHY2lr22hYlXZuMI66CyEb140e6JmH2jfIS8r9gANQWd8F8UK3PYXkWKJKKmIbL50Irnmq1xfRraXYB+e/j8iJ4/6Qi9Sx8H2CSaw3zDpI6i6fVPrvSaVxe+UT0T3sWM3Hzr4ifmsNMHjS3zMU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1775068178; c=relaxed/simple; bh=KPXI20iVVZNFRX5voCux23f5vb7U3aHflMkahG64EI4=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=DGSrd5hICgmaqKWLN4SX2pK8Sx9IySS9Eu/KT02R8KoVTaK+tPN/j+hzuGMppTg/o106EJkuXq3F88Sv4nYOO2wyjeVpAzrwnHEpok2hPMoIHXqQbza/Q9Oe9Fqcqk2H+v8hVlGS1fItpxCHUSyScS3aTmrZJCPfMUj/U79sF80= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com; spf=pass smtp.mailfrom=gmail.com; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b=cnG2+7Lc; arc=none smtp.client-ip=209.85.128.48 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=gmail.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=gmail.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=gmail.com header.i=@gmail.com header.b="cnG2+7Lc" Received: by mail-wm1-f48.google.com with SMTP id 5b1f17b1804b1-48704db565eso187925e9.1 for ; Wed, 01 Apr 2026 11:29:36 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1775068175; x=1775672975; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:from:to:cc:subject:date :message-id:reply-to; bh=kiRZsxMjtLkEkAWwlyx0ty6e7+uRzHmpLeHb7fLvvgg=; b=cnG2+7Lch5AvcgKWjNftu13k4XO1pBaSMtK4N6yRRgQFGHl5RVtlDtBMaarQQCIO1w D7dmJNsijc5kELsFGICehmkDbUsg9BcR3cDsXRsX+kZwFV9UQiVlaaSlAe5n6VntGNYH /i79fsFFITZe139A7jJwGNEZiOZ2Rr1SFft7jH2j5pfL0GZCgT+Cik0ATwNpyL9n6BAi Akg8sIzNHv4FhDp/DLDEp94eau0lI92mYdy+MvPisiqE8k78MOl3xOiSkLo0Wr9lAPWI OV4Kno6DDrfe4GzOFsHyWZ96Hi+1Bj93MnddTHpDlMJ0wJVh+Fwys82d/9a25RrPkbRM 1UtA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1775068175; x=1775672975; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from :to:cc:subject:date:message-id:reply-to; bh=kiRZsxMjtLkEkAWwlyx0ty6e7+uRzHmpLeHb7fLvvgg=; b=mGO3aWtkvdZU1JqjBNTzetWyiH+GmAGffkXm40wzbcg6daXxnNE1JpfUqidnqB4Pf7 vibH5+pJadW5F0b2uJXIGE6BfvG8YTIGMLF8oycEuLxu915SlP94t0S7OXT5NZGW9DLO ron/b50NJ+f68RAMy4clbtT1lDAyeu48z7aOGQUM4iOItoUA8o3shGjYdy4wp7c1P+x4 ye4i5zfZc8owGCAfNytsgWRwli833/NGgQWltwXrNbBeNHaELG8gc+T9fUW+b1+VcMr9 0BF/9DqCR/8t3hflbfjkzWDogKSiIDnvW1M6ju4VyuwIQ/6Qb60MsqmzTboFZEr9xK63 2Cqw== X-Forwarded-Encrypted: i=1; AJvYcCX6FBK+qqLluRjtrVnSAgt6wICAzGGWiKZjoNOBAwQftIPmR+SScWwGKe6aW7zH+sB/Ur5Lk7aM9rwCvTI=@vger.kernel.org X-Gm-Message-State: AOJu0YxZ0XVPAD9k9TA9ecyJ53YUUm6USRVufg8latV/lB54HXenm4u5 rhflpzij6FActIj/jjcRY8ZYNx1vVQm1Ga+c+JvLxuHxP24BwLbiEaFfoPgEiuNz X-Gm-Gg: ATEYQzy2JlvfFZA+0cVoeJIOEaQKlBW89MuYlYkHz36m06dMW2cLbGC61rzC1PXOuFa veOP9bq7daXtI933KlpVpbjNECXcidAXJRtkAzxvG4NHUdQMCXHowfYS93h0cf7KwEqwbP1edbs IOlqG+w01Qa9gydIerCHyKDBRD7xU08F6MnBnsAUq8v2wv79B+yCOxgI3yYZdpIk4xS+I/Z4zEu Wo8XfizqjoCxNNOTylaLT9y5vnWS8aPTTKn9GuGuPpRhVYjyaO+E3zHg4gTbjaRxkUaSXeb+I8I /KCXN3wgK2uyuaaqzDEx4twK1u3UJKjSRZBk802UP5S25vQBqO5Ck+rGGer5MiAaraZvLt8F8R8 q6IE8E4sdHH13X5SNmfW6IKN+JYj6qf+5RPels4sWQg24DAppH8hjVj6p3WO+htcvYe8O1gJID7 OsVUJmSulUwMT/l9MztDjLl6cJxoSHu2kM4dQXc1ZgYukZuTEstHnfMmq7rhpu X-Received: by 2002:a05:600c:1e8a:b0:485:ae14:8173 with SMTP id 5b1f17b1804b1-4888b6eab06mr5725665e9.1.1775068174700; Wed, 01 Apr 2026 11:29:34 -0700 (PDT) Received: from pumpkin (82-69-66-36.dsl.in-addr.zen.co.uk. [82.69.66.36]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-4888a62628csm14742635e9.2.2026.04.01.11.29.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 01 Apr 2026 11:29:34 -0700 (PDT) Date: Wed, 1 Apr 2026 19:29:33 +0100 From: David Laight To: Petr Mladek Cc: Andy Shevchenko , Josh Law , Steven Rostedt , Rasmus Villemoes , Sergey Senozhatsky , Andrew Morton , linux-kernel@vger.kernel.org Subject: Re: [PATCH 3/4] lib/vsprintf: use int for field_width in vsscanf() Message-ID: <20260401192933.6a854399@pumpkin> In-Reply-To: <20260401152256.0b11e6c1@pumpkin> References: <20260324224940.50508-1-objecting@objecting.org> <20260324224940.50508-4-objecting@objecting.org> <20260331163522.74467342@pumpkin> <20260401152256.0b11e6c1@pumpkin> X-Mailer: Claws Mail 4.1.1 (GTK 3.24.38; arm-unknown-linux-gnueabihf) Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit On Wed, 1 Apr 2026 15:22:56 +0100 David Laight wrote: > On Tue, 31 Mar 2026 18:12:18 +0200 > Petr Mladek wrote: > > > On Tue 2026-03-31 16:35:22, David Laight wrote: > > > On Tue, 31 Mar 2026 16:31:50 +0200 > > > Petr Mladek wrote: > > > > > > > On Wed 2026-03-25 14:00:17, Andy Shevchenko wrote: > > > > > On Tue, Mar 24, 2026 at 10:49:39PM +0000, Josh Law wrote: > > > > > > vsscanf() declares field_width as s16 but assigns it from skip_atoi() > > > > > > which returns int. Values above 32767 silently truncate to negative, > > > > > > causing vsscanf() to abort all remaining parsing. This is inconsistent > > > > > > with struct printf_spec which uses int for field_width. > > > > > > > > > > Is the field_width an acceptable integer range by the specifications? > > > > > > > > I am not sure what is allowed by specification. Anyway, the code is > > > > not ready for a bigger values, for example: > > > > > > > > case 's': > > > > { > > > > char *s = (char *)va_arg(args, char *); > > > > if (field_width == -1) > > > > field_width = SHRT_MAX; > > > > > > > > clearly expects signed short int range. > > > > > > > > I wonder if it might even open some backdoor. The code matching > > > > as sequence of characters expects a defined field width, see > > > > > > > > > > > > case '[': > > > > { > > > > [...] > > > > /* field width is required */ > > > > if (field_width == -1) > > > > return num; > > > > > > > > The current code limits valid field width values to positive ones, > > > > I meant this code: > > > > /* get field width */ > > field_width = -1; > > if (isdigit(*fmt)) { > > field_width = skip_atoi(&fmt); > > if (field_width <= 0) > > break; > > } > > > > If we change the type of the local variable then the above check will > > suddenly accept field_width <= INT_MAX instead of SHRT_MAX. > > But it is all broken anyway - consider "%65537[abc]". > The test needs to be: > if (field_width <= 0 || field_width > MAX_XXX) > (and I doubt 32767 is a same limit anyway). > > (oh and skip_atoi() need to either saturate or stop consuming digits) Actually I realised later this is all pointless. All the formats are literal strings in the kernel - they are known to be ok. I think even the (field_width <= 0) test can just be deleted. The code won't loop forever, it might overrun the output buffer but than can happen with a syntactically valid length as well. David > > David > > > > > > As a result, The above mentioned "case '[':" handling will suddely > > allow to iternate over INT_MAX long string instead of SHRT_MAX. > > > > I doubt that there is any kernel code which would be affected > > by this. But I do not want to risk it. > > > > > > aka SHRT_MAX which is clearly much lover than INT_MAX. And it might > > > > prevent some out of bound access. > > > > > > Best Regards, > > > > Petr > > > > > > > > > > > > > > Notwithstanding what the code actually does there is no point defining a > > > local variable as a 'short' unless you really want arithmetic to wrap > > > at 16 bits. > > > All it does is force the compiler to keep adding code to fix the sign > > > extension to 32 bits. > > > Look at the object for anything other than x86 (or m68k). > > > > If you think that it is important enough, feel free to send > > a patch. > > > > I not taking this patch from Josh Law, definitely! > > > > Best Regards, > > Petr > > > > PS: Note that Josh Law seems to be an AI virtual person, see > > https://lore.kernel.org/all/cbd0aafa-bd45-4f4d-a2dd-440473657dba@lucifer.local/ > > > > I am even not sure what to do with the other 3 patches. They look > > correct. But I should not take patches with an unclear origin, see > > https://lore.kernel.org/all/2f824be3-7b30-41c6-b517-de1086624171@kernel.org/ > > > >