From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from galois.linutronix.de (Galois.linutronix.de [193.142.43.55]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 8F273343D7B for ; Wed, 25 Mar 2026 14:16:31 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=193.142.43.55 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774448193; cv=none; b=iQuAInHFuiqNEWQ23ghq0erWiqEzXZcNA++8efcMj07boDhZpnEArZ0QMeRdTuJ77PylmMIkSGH6F5YdYem3rGDL2GgfWZoyeKqC2mGFIk1rQ3XNyFWZJaSrdskLLXABSzpe9VaKHXgzCShGXf40d+28hmNa+XlHotzUVdYqAbY= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1774448193; c=relaxed/simple; bh=3rUxhjwo2kLaNHtoB6ITIeSZde2vuVO1ei46PBObm38=; h=From:To:Cc:Subject:In-Reply-To:References:Date:Message-ID: MIME-Version:Content-Type; b=ThgtImZTn32hf6mW9HGAn1Kldv2tDvxIK/x8S5XXIyC1nexsLSKGSjDRxCh1deLGe4cS6ZkFdZ3xCQd8gjLIHwiALtpa3ndEt0P26tvbyhjyZ6azoLgQs6uyDOVuFHM2sQoYaLhLnMxhFijbcldPMyLdGevONu8/juwsSY/GOz8= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de; spf=pass smtp.mailfrom=linutronix.de; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=T0yq7Eca; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b=jJUOJiQU; arc=none smtp.client-ip=193.142.43.55 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linutronix.de Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="T0yq7Eca"; dkim=permerror (0-bit key) header.d=linutronix.de header.i=@linutronix.de header.b="jJUOJiQU" From: John Ogness DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020; t=1774448189; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=vB2mcxuLMPEd8vyVYGRKE6+YDuMHEERvSzMW6Q8QYxQ=; b=T0yq7EcasMWN8aGj5l0Wb7eZfgqHRFDAfCBc1FDsn90GBoUUnyVB5IKspOAMv+bZ4d7CJn 6x5ZcTYtY/htyJ8XGbsJzLcG+ERkgtjFhwTZypZ5bueQx3/P/dJB5lW9G6j+2/5T7qdhsJ PA9sh5KU2M1jk3yFYSTB9PxXvW/D1Cx5Utu2s7ws4bvI/VcKvS92QB9bfIhwCtaxMotBJq 9BscwvNAlURiElRWp5G9xOx30dNYQXpU2S0PjqocyFfIZ4ik94rN5yJoUqqJ3SuOl8szaE kRaQuR3R6KgTcINYkDxBwAVHHFcGG8MQWrCk7ihcOSkTGeOHtAUEdjfL+x3GPg== DKIM-Signature: v=1; a=ed25519-sha256; c=relaxed/relaxed; d=linutronix.de; s=2020e; t=1774448189; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=vB2mcxuLMPEd8vyVYGRKE6+YDuMHEERvSzMW6Q8QYxQ=; b=jJUOJiQUmPTUG8EHfRjdVFPaij4W7c4/8W0YRTwGWoIL0T/jWX1HwkvaGMWzpxm5AWBHy2 KdriKgu6selgFUDA== To: Petr Mladek Cc: Sergey Senozhatsky , Steven Rostedt , linux-kernel@vger.kernel.org Subject: Re: [PATCH printk] printk_ringbuffer: Fix get_data() size sanity check In-Reply-To: References: <20260319134955.185123-1-john.ogness@linutronix.de> Date: Wed, 25 Mar 2026 15:22:29 +0106 Message-ID: <87fr5o570i.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 On 2026-03-25, Petr Mladek wrote: >> --- 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. It is not wrong. It is checking something else. > 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. Well, the point of the WARN is to catch when things are broken. If for some reason a data block of textlen 0 exists, it is a bug. I think we should still catch it. I do not know if there are places in the code that assume if it is a data block, it must have a textlen > 0. Probably not, but still, it should not exist. John