From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f53.google.com (mail-wr1-f53.google.com [209.85.221.53]) (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 A7E973BB9F8 for ; Wed, 20 May 2026 22:12:01 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.53 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779315123; cv=none; b=Z8yEx59FZ4upmbMxodI/vlOdHaFomSadArNUKZkKyEJyS/wNczevWvGhf/NupreX4B1X7YMVxGoX7QlGqZ3/IQSz6fkiBIvhHNBFQ4kuOn7GD3dBbsdOOwpQOTTFVqXlL5JxTaChNT6xrlfFaMzltaKcgboiowg2cDsNmgz80Hc= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779315123; c=relaxed/simple; bh=Cof4Jn9Pb0lNFELLArIlY7ZcHjIiQKqHlG/bd8/G9F0=; h=Date:From:To:Cc:Subject:Message-ID:In-Reply-To:References: MIME-Version:Content-Type; b=sZ+2TJCGPd4VPRgtHCtLcM+Py+QVR2T3f3kQw9XqRR1HkeCggqgmBKjV8hke4XZkUB/upAHy3HMor/BvtwjapG7jVuQbk3PzBqsHJa6y8/H5YwtZNV386iQu4DLsAgbEjH6x6q/Ycab3am/sPzvhEXsOV+GIAcBOLvQhyvd2vlk= 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=hAql5mfJ; arc=none smtp.client-ip=209.85.221.53 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="hAql5mfJ" Received: by mail-wr1-f53.google.com with SMTP id ffacd0b85a97d-43d77f6092eso3122343f8f.2 for ; Wed, 20 May 2026 15:12:01 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20251104; t=1779315120; x=1779919920; 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=kRpSg0hy3Rjf3yEZPX+t8O4volQZbZItPIjeFWyTS4I=; b=hAql5mfJZXhlkIINlIk12ESFVzTa8w7W8KzIyQXt8da0f6MR2L5xJCCRwl2bZmAL9e OmNpz9kAvG49zZ0IMq9X8hAOB3DK2n3XI9iqtklKvK/1oTn0evWC+BWDYqq34PbwGGG4 LOcovPngmo05+TfPlTRj+dcv3fDHTc91I8BCLO/nGhTd36CwldOFnaqHHM2cFQ8b0ap8 rDSpyrSklqzqKMPvSKombOHO9zYqBdg/5gsnx3Fhp8toloer4dvhWiWwlh4QLuQaf4tQ m4fQhegjVqAfmaadtvZ1DENlsIuK343++hHRhirlqZbC8QMMrzJoTPz53INne8VFrc1l bh+A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1779315120; x=1779919920; 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=kRpSg0hy3Rjf3yEZPX+t8O4volQZbZItPIjeFWyTS4I=; b=UMhyo7K/1AfDXmTjtTJo3xq7DfbDy+V/g4NM/pkTRKIF8XyGdm6cqGUsaPPAwUY/0G eKJFIFvuAlWhxwDXHyHkGSB6qzlPFfcCCh1+bDUdK8OXqsADuqncgtYQt8QR+HvwYdAd pryZAYeKbN15m86syXwjF/zl87sIXMye3w58eNvokhyd6igAzkYY3HR2I91Z72c/an+j MrF+O7jpqIRN2OXTvFsqy4SGMTsdTzAysboswyu5/G8zJ0+grbE2kICLKGzFE/f0ObmV x4fY/j9KLZOSf8WR2zGflFxVTl0BzAQZd5pgMRKmvSZM+ybqywsmQh8AI3Yj5SF2JaEy P5CQ== X-Forwarded-Encrypted: i=1; AFNElJ8cwvKw+1SCUCFQueQUW2D7nzjKU5AkHd+cVmohAB0UNOJawWFxtGcM4p7F7VoULpOKgE6c74O2PELyTB4=@vger.kernel.org X-Gm-Message-State: AOJu0YyA6vFcTmMZ7rkgZBVbcikFdskcOurRIVF4AdMPwDWjCaCD83jS 2wZ62tKKUVcM3Q8OflJjLk0XIL11L1gQxx6rGhqFjnB/gtV/zW/JK0Mq X-Gm-Gg: Acq92OGai1271EHYKmAXo2oayh+ojofFIjKZgSEq1x4OHHVjGDygZxjybwH9sLjUF0L xEwAs2TJDEeFaaqJmz4cD9iE0XWeSuKny5NlDlg8HlWhOGmH8Xw2YPheWSZC3joKCPA/CelNqoM +9NYR09xd6hlUTsHkPsw2b/p9HIFKyhz5VJ8k2hf+ETP0CPlpkHo1X+8pUTcymR4R+cADSkbY3X EUVx7Sp6gmm1wCvfTtO0k6xhoIOFBgDyGl2mobi8MbJYaAPbc4yngAym0TdlZq9OO9apSGIH6Rz 3U/aXdNcU/SrU4tzofEbn3AzqAEfsxXESoV7FIURS6H3/9Agb+fXMOvntilNpdq3yoIRC0fViZr a8iJL837fSyKiHqniRx19yURF5xrdDajfB6dH0rgmc9QUXZ5JOzENnQQrgu9M0s19TTX/HTdfLn OHRlz+3sYRkAss+KVd6JQlf8BZghDV0ZLh6hL6olTxOm+DKLvkdSxHb+8k+2nrElpW X-Received: by 2002:a5d:6f0b:0:b0:45a:cea8:e4b2 with SMTP id ffacd0b85a97d-45ea410e9ffmr196007f8f.29.1779315119850; Wed, 20 May 2026 15:11:59 -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 ffacd0b85a97d-45da15a666fsm58894877f8f.36.2026.05.20.15.11.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 20 May 2026 15:11:59 -0700 (PDT) Date: Wed, 20 May 2026 23:11:58 +0100 From: David Laight To: Greg Kroah-Hartman Cc: driver-core@lists.linux.dev, linux-kernel@vger.kernel.org, "Rafael J. Wysocki" , Danilo Krummrich , NeilBrown , Tejun Heo Subject: Re: [PATCH] sysfs: clamp show() return value in sysfs_kf_read() Message-ID: <20260520231158.407ebf0b@pumpkin> In-Reply-To: <2026052000-drove-unicycle-d61b@gregkh> References: <2026052000-drove-unicycle-d61b@gregkh> 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: quoted-printable On Wed, 20 May 2026 15:07:01 +0200 Greg Kroah-Hartman wrote: > sysfs_kf_seq_show() defends against buggy show() callbacks that return > larger than PAGE_SIZE by clamping the value and printing a warning. > sysfs_kf_read(), the prealloc variant, has no such defense. >=20 > The only current in-tree user of __ATTR_PREALLOC is drivers/md/md.c, > whose show() callbacks are well-behaved, so this is hardening against > future drivers doing foolish things and out-of-tree code doing even more > foolish things. What is the rational for it using PREALLOC? =46rom the code it looks like it could set a buffer size, but it doesn't seem= to. Which means there is a kmalloc(PAGE_SIZE + 1) in there. If it did set a size, the check below would be wrong. =46rom the look of the fragment below it is just passed the address of the pre-allocated buffer. That also means it can't use sysfs_emit() because that relies on a page-aligned buffer. Also I suspect that kmalloc() grabbing a buffer from the per-cpu freelist is cheaper than the mutex required to lock access to the preallocted buffer. It would be 'nice' to change the type of the buffer passed to the show() functions and to sysfs_emit() to something other than 'char *'. Even if the initial change is just a typdef for 'char *' so that you can tell which functions can call sysfs_emit(). At the moment it is pretty hard to actually decide whether it is safe to change the printf() to sysfs_emit(). If all the show() functions are expected to include a terminating '\n' then the wrapper code could add one if missing. That would save a lot of strcat(buf, '\n'); return strlen(buf); sequences. I also think (bad for me at 11pm) that the buffer should be 4k not PAGE_SIZ= E. -- David >=20 > Cc: "Rafael J. Wysocki" > Cc: Danilo Krummrich > Cc: NeilBrown > Cc: Tejun Heo > Fixes: 2b75869bba67 ("sysfs/kernfs: allow attributes to request write buf= fer be pre-allocated.") > Assisted-by: gregkh_clanker_t1000 > Signed-off-by: Greg Kroah-Hartman > --- > fs/sysfs/file.c | 4 ++++ > 1 file changed, 4 insertions(+) >=20 > diff --git a/fs/sysfs/file.c b/fs/sysfs/file.c > index 5709cede1d75..25b44fe171a3 100644 > --- a/fs/sysfs/file.c > +++ b/fs/sysfs/file.c > @@ -120,6 +120,10 @@ static ssize_t sysfs_kf_read(struct kernfs_open_file= *of, char *buf, > len =3D ops->show(kobj, of->kn->priv, buf); > if (len < 0) > return len; > + if (len >=3D (ssize_t)PAGE_SIZE) { > + printk("fill_read_buffer: %pS returned bad count\n", ops->show); > + len =3D PAGE_SIZE - 1; > + } > if (pos) { > if (len <=3D pos) > return 0;