From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f42.google.com (mail-wm1-f42.google.com [209.85.128.42]) (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 962EC24BD03 for ; Wed, 25 Mar 2026 13:56:28 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.42 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774446990; cv=none; b=EzlOmEZu54U1+hRMPApBoNR5QiSAxT/d/Q1fVocIo2JBHBII9dRa6NDd7+zH3HIUPGhGS0H2T7+nFjcnF3liWAiGfuOO91b8KeIaSZtdcDljrdJT7F+fEGOZV6mhyj3KZhckiblMiKKUqmmV6ThlkKt587KgRbLj0QyxGUeYSoI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774446990; c=relaxed/simple; bh=GTe5IA/ys/fjaWkIRqPCIyH2ClmlnCn3XF+iuv3Xq7E=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=WL32kQHQYOi4golofQ0w0LWDkkQ7WTqeA6MwN5aQM60nZm2d4d1vQMOXz+R08mR/lR/WohSZgjCe6vu33fw7LrYs1c+RPVbbeEm5NIWQUmREXzRJSQSrwu2p+dSpgy2zzHdILgnAgVRbIQqOibLQDlkuVt09IBNcrfwC08AmagA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=V8xhXs8z; arc=none smtp.client-ip=209.85.128.42 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="V8xhXs8z" Received: by mail-wm1-f42.google.com with SMTP id 5b1f17b1804b1-486b9675d36so54699395e9.0 for ; Wed, 25 Mar 2026 06:56:28 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1774446987; x=1775051787; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=Y3jVxjAKYlwjPFHYqDkRnpp8igDbqds/XhSToVVRCvw=; b=V8xhXs8zgY2K8CQIpdSKWTTOYM6IQTirEj3gdjBxVgvENhpeKbf08NlC3ViQeqkV6k kRyZvSp0VRbqhXl6mP6hsRYbZnI+Ct5xE6V5yBLnCI/9L2Bpk+qU3DLd6MaZRWXQ3+OY +h0I74rKNMp4Uo1PKk6n0Fkj5DGHJpDmpcCCZ3TKtRzBt8bGkiSCw760cEW9+F2D/7Ip dP4hRFjrav/RsMlU+YfMbpfz/pdiCZo8XZ0Ybd9qi9BvGQsCd9vn+iCO0gD990lwd/Ck 8O6yHStf3PIShjjDbvJZd8RkDyWd8RZHIS2zC/05Kj5tI4kfiWAZOu39M49b2Kh8znsL CQFg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774446987; x=1775051787; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Y3jVxjAKYlwjPFHYqDkRnpp8igDbqds/XhSToVVRCvw=; b=Ru0/VQY2WYjklnDUm4VMR8jMiVCFbajBx2U9fDdiLKLPukmCXw9wHaAKAyVtamf4cR f5UXMyXaTp6uCwxtbjFsqGLfRE+VnUAW+kyfLLXSdBjDxo+0JBfWX3nnX6LJKqZT/dke 2CCZjs4cxxEOcayTMbRERmnGqRSgFyYClMSsGKGNthwfULqJpv+7Ycj87lX3OkIz7Nid NUwvS5B0E5/F1ZLisdt6bL/Z5dSfA2uQqI0e4HodUmGZ3YIQRwC4Drhc4TMXBJwcGqL+ fHBNNEWXviIyFEe4ETH5S4RHRK9VUINJ2aeJPpuvHICH2Yf7GRXZh+5J0KYyfmqGBrQ2 RURQ== X-Forwarded-Encrypted: i=1; AJvYcCWXCTb3/Ajann86sGfYm98/9rjQQ3a66ZtSJ8v5lbr2Y6DZApKRKVxtM3vnNp86dsMgy13y93x7jAjvD74=@vger.kernel.org X-Gm-Message-State: AOJu0Yw99FYeiAjk+XChJXWrtsCDba3Z6sNaitx6pcyc5RBdSRAamAel J+QQdifrqplqqsy9Z37Lz8fyc14kyAVC+aFfLg8NJvZ5q3NXtwwOhgs3JZl8FxkL108= X-Gm-Gg: ATEYQzxFWdD4cyHqCex6imUFxszs6tYQoV3pyPk6GhsXU5C9dshzv+p9WA0jjU8DIiK x2iYiAh0peRLVte6Fqjsx/9c131VDaHc7lLFQO5jiugvLxIIc9X9EM1yv7br8KH2OPTOApbBeEZ Uz6E1Rar5FWWusvFq1dsKpC4OTBl3W95MHhqykX4V+e1eNJCvyGDH/TsAaE+liKZxZzuAw5rGaP JUAaiGfKf5eNlEwKe61H1c+Fm6xauc3GoksIISuVZTEQgzdB17DUgW/sf2+n4xpJIw4tGyAxCcm j6LLIuKO9JqtMOsVcqftCopWSSAJt2SJXhyxP/UOA97TYCObHJNDbNGkmmGZpT6ZWbPK+7x98zq Zt54LT6jrQbl8ipUtPq8k+DDKoyHQlu/At2uKko299O0v0dSZUE2QeyiB+OH+molLsxUZCPTeJP mKfgmlwQ0sO5Zf+wOz7HcxUI1Enw== X-Received: by 2002:a05:600c:4692:b0:485:4eaf:eb54 with SMTP id 5b1f17b1804b1-48716039b83mr53266745e9.20.1774446986913; Wed, 25 Mar 2026 06:56:26 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-487116939b8sm204473275e9.3.2026.03.25.06.56.26 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 25 Mar 2026 06:56:26 -0700 (PDT) Date: Wed, 25 Mar 2026 14:56:24 +0100 From: Petr Mladek To: John Ogness Cc: Sergey Senozhatsky , Steven Rostedt , linux-kernel@vger.kernel.org Subject: Re: [PATCH printk] printk_ringbuffer: Fix get_data() size sanity check Message-ID: References: <20260319134955.185123-1-john.ogness@linutronix.de> 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-Disposition: inline In-Reply-To: <20260319134955.185123-1-john.ogness@linutronix.de> On Thu 2026-03-19 14:55:39, John Ogness wrote: > Commit cc3bad11de6e ("printk_ringbuffer: Fix check of valid data > size when blk_lpos overflows") added sanity checking to get_data() > to avoid returning data of illegal sizes (too large or too small). > It uses the helper function data_check_size() for the check. > However, data_check_size() expects the size of the data, not the > size of the data block. get_data() is providing the size of the > data block. This means that if the data size (text_buf_size) is > the maximum legal size: > > sizeof(prb_data_block) + text_buf_size == DATA_SIZE(data_ring) / 2 > > data_check_size() will report failure because it adds > sizeof(prb_data_block) to the provided size. The sanity check in > get_data() is counting the data block header twice. The result is > that the reader fails to read the legal record. Great catch! > Since get_data() subtracts the data block header size before returning, > move the sanity check to after the subtraction. > > Luckily printk() is not vulnerable to this problem because > truncate_msg() limits printk-messages to 1/4 of the ringbuffer. > Indeed, by adjusting the printk_ringbuffer KUnit test, which does not > use printk() and its truncate_msg() check, it is easy to see that the > reader fails and the WARN_ON is triggered. Uff ;-) > --- a/kernel/printk/printk_ringbuffer.c > +++ b/kernel/printk/printk_ringbuffer.c > @@ -1302,10 +1302,6 @@ static const char *get_data(struct prb_data_ring *data_ring, > return NULL; > } > > - /* Sanity check. Data-less blocks were handled earlier. */ > - if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size) || !*data_size)) > - return NULL; > - > /* A valid data block will always be aligned to the ID size. */ > if (WARN_ON_ONCE(blk_lpos->begin != ALIGN(blk_lpos->begin, sizeof(db->id))) || > WARN_ON_ONCE(blk_lpos->next != ALIGN(blk_lpos->next, sizeof(db->id)))) { > @@ -1319,6 +1315,10 @@ static const char *get_data(struct prb_data_ring *data_ring, > /* Subtract block ID space from size to reflect data size. */ > *data_size -= sizeof(db->id); > > + /* Sanity check. Data-less blocks were handled earlier. */ > + if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size) || !*data_size)) The check of "!*data_size" is wrong after sizeof(db->id) subtractions. The question is whether we still need it. As the comment says, the data-less block were handled earlier. And it seems that data_alloc() explicitly creates data-less blocks when the given size is 0. And prb_reserve_in_last() only allows to increase the size. IMHO, we could remove it. Wrong values will be handled by the above check: /* A valid data block will always have at least an ID. */ if (WARN_ON_ONCE(*data_size < sizeof(db->id))) return NULL; A zero size datablock which is not handled using the explicit data-less block is not expected after all. Best Regards, Petr > + return NULL; > + > return &db->data[0]; > } > > > base-commit: 9095f233c0258e9a05e958c7d822eb38681e7a5a > -- > 2.47.3