From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f48.google.com (mail-wr1-f48.google.com [209.85.221.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 1F8393BF67F for ; Thu, 26 Mar 2026 12:20:19 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.221.48 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774527623; cv=none; b=hc9irCjcwELGEDRSRx1WnNAd37KfP0rJU+lV+sin9+2eMyJJ5cLaRU2FiXh5/cshASE5ELR7HHhc/MeAvWX0pBgJIL7LJN+/5m0Sbs9BUNu6Nkp7Lq7RCt9ZKFYQfUoCyWdGs5LtGDT4MmOSjj0Cl8V8nQQmHBBd8XtfBuCy4wQ= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774527623; c=relaxed/simple; bh=+2n1xB6z5ewe71TYgpWzNvAaMBvToXHHXcFZvEPD9+I=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=SF1Hg5EkcH/S/vPgQRe68MTCLkRb057gYWZ9E4IHbA89QO8NUViHa08oh2mXsD6iMHAGSuXEkIEXJl7SYSadNQJMaCaSv0nvMhRHGlIp2teNCEQ3I3T3Z4QFUjGqg18/PAu4ZY+3Uvt7FCaOGrz/FzjnA2DVzncqE0fxkFiwL2w= 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=AcqnyBzG; arc=none smtp.client-ip=209.85.221.48 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="AcqnyBzG" Received: by mail-wr1-f48.google.com with SMTP id ffacd0b85a97d-43b88b7ca76so696386f8f.3 for ; Thu, 26 Mar 2026 05:20:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1774527618; x=1775132418; 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=OdYHrH/p93mA10ihaMIiSpJvDvGHyhSIyZjacr3rx3A=; b=AcqnyBzG2mBjhuoxOaCIaSOe0UpJbg690LKPQgw0coeKwgvgHcZl5MIq8PG59I5P2c OfFQMyU4hm0PWxXEwoIQztXHaEq1rsuvs0USg68DxV/oTYIlpdmP9zlGPm25KoDRz9Yi osEnaDxaVJHofmLxaczYtRtReZ5Pq1605wy+zOBfW3lL4B4Ko3iepYQbKmWowsmuWMoi MMbwCzswNFn4h7TjaGoiCae0CtoDCCjj14K9KYz/6fjcLs84vxKXG8NRLsRBl+8jxyu0 t8EWmy6qynv0SRUDns+gNXG8GHwQMl4d1t8tcDF9lZeC8hMZ6P2KVSWx4VkgOem4ty/+ 7lwg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20251104; t=1774527618; x=1775132418; 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=OdYHrH/p93mA10ihaMIiSpJvDvGHyhSIyZjacr3rx3A=; b=cAxr0+enaoal+41Nln4qntPmaMXSz0pQYVu24C0jKnN9al7rIsbZu3RiukuRjMTfOI 6bgF9C/Dh+owBhz91OILioyGml+OcpbHm81kWZDH147cq8u5gCVBp7eC+G4e7BibOZVB McsbrveYM3S3lxQPSprQX4VG0KCtXljq4+NJfVDSpezr0Jw9eNTqb/4sT1kAgVgY5Lhb fcR1kuQ9gGWaCKLmUZolOoT5fZj0m/5bipU4OaY7SkPzYvz07DRiz0rkHpPn7lapbaT9 scLoH+xYBDce93f3bZGE8Qm+jwPcIRhemy+GvjE6Mo7LKtVjmAkWGnh2i+4RkhsIB3En VdTw== X-Forwarded-Encrypted: i=1; AJvYcCWA0m7mjOJ9EFrZTEUXXVGg+n0dmdbSCVoSH2UkIPJw7bZC73sJW5zQvJsd1WpEhnGVJhgc5U28X+IflOQ=@vger.kernel.org X-Gm-Message-State: AOJu0YxoMv5jFySvwnzpQ9uWhJZLJdTYQZEH3Cw/i3P0+RcmogUZkVSl nzY/cp3f6g67SufbGGLe+VODlK2ZPltzqBIoCYFFYT4p1Ye6EBQGVml4jUPbYXwxLuA= X-Gm-Gg: ATEYQzw21sjTcji697JYMj2cKLnrudANjv1I3FRYElsQ0ylOAd5HxNiNRRRrG5IBsC+ wptxtyAjl1ZZPsyKrQ7Q7kIcPmB3hUgaG0v6B1Ybl7TcHVsLhJenv1LUp6Gt6atRGGxuogIdY94 4d+jkO1/CU8h5s2DY48DlyhapnUXJi6PRbUzvXLeRCWuhXGSv+Kc7DtnIVIoKZeb90P5ZGd+gCF tX43D/MIu3CLtTMjAkZEq2le2LpDpo8xZ8U7J1hj0TNoO6djPHwObWwFtNAexaJLyRi/rgckfZ1 0r6tToWQlzfJF4snOHzZeqIA54+vHUy/DIcEjGYb08hVZswnyM08oEGf1oVzWyM2Eh+F+DznHQT pbWBkKH92K4mBFfYTSss2HO2/dMlDA8r0k/I9dvM9kGssvPFzqPVHxgffX9iNzkwQcwbnTrk6Rd HjqfGi7JVhvyFQCUnK7ORqWU0sag== X-Received: by 2002:a05:6000:3101:b0:43b:3b80:6776 with SMTP id ffacd0b85a97d-43b889e34edmr12075462f8f.30.1774527618328; Thu, 26 Mar 2026 05:20:18 -0700 (PDT) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id ffacd0b85a97d-43b919df903sm8047490f8f.30.2026.03.26.05.20.17 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 26 Mar 2026 05:20:17 -0700 (PDT) Date: Thu, 26 Mar 2026 13:20:16 +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> <87fr5o570i.fsf@jogness.linutronix.de> <87cy0qyitr.fsf@jogness.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: <87cy0qyitr.fsf@jogness.linutronix.de> On Thu 2026-03-26 11:46:48, John Ogness wrote: > On 2026-03-26, Petr Mladek wrote: > > So, just to be sure that the new code works as expected. > > Note that the moved check: > > > > /* Sanity check. Data-less blocks were handled earlier. */ > > if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size) || !*data_size)) > > return NULL; > > > > warns only when data_check_size() fails. It just quietly returns NULL > > when *data_size is zero. > > ??? The above code warns for !*data_size as well. Grr, I wrongly interpreted the brackets. > > If we really want to warn. Then it would make more sense to change "<" > > to "<=" in the previous check before the subtraction. > > Yes, actually I would prefer that. Let me send a v2 where I relocate and > reduce the data_check_size()-WARN and extend the data_size-WARN. So it > is something like this: > > diff --git a/kernel/printk/printk_ringbuffer.c b/kernel/printk/printk_ringbuffer.c > index 56c8e3d031f49..aa4b39e94cfa2 100644 > --- a/kernel/printk/printk_ringbuffer.c > +++ b/kernel/printk/printk_ringbuffer.c > @@ -1302,23 +1302,26 @@ 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)))) { > return NULL; > } > > - /* A valid data block will always have at least an ID. */ > - if (WARN_ON_ONCE(*data_size < sizeof(db->id))) > + /* > + * A regular data block will always have an ID and at least > + * 1 byte of data. Data-less blocks were handled earlier. > + */ > + if (WARN_ON_ONCE(*data_size <= sizeof(db->id))) > return NULL; > > /* Subtract block ID space from size to reflect data size. */ > *data_size -= sizeof(db->id); > > + /* Sanity check the max size of the regular data block. */ > + if (WARN_ON_ONCE(!data_check_size(data_ring, *data_size))) > + return NULL; > + > return &db->data[0]; > } I like this. Thanks for patience with me. Best Regards, Petr