Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <d0798b26-9c77-7209-8c16-0d067ea5720c@rasmusvillemoes.dk>
Date:   Fri, 17 Jun 2022 09:15:53 +0200
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101
 Thunderbird/91.9.1
Subject: Re: [PATCH RFCv4 3/4] lib/test_printf.c: split write-beyond-buffer
 check in two
Content-Language: en-US
To:     Petr Mladek <pmladek@suse.com>, Jia He <justin.he@arm.com>
Cc:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Rasmus Villemoes <linux@rasmusvillemoes.dk>,
        linux-kernel@vger.kernel.org,
        Kent Overstreet <kent.overstreet@gmail.com>
References: <20210615154952.2744-1-justin.he@arm.com>
 <20210615154952.2744-4-justin.he@arm.com> <YMtZcVy4gvmMtYv+@alley>
From:   Rasmus Villemoes <linux@rasmusvillemoes.dk>
In-Reply-To: <YMtZcVy4gvmMtYv+@alley>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 17/06/2021 16.17, Petr Mladek wrote:
> On Tue 2021-06-15 23:49:51, Jia He wrote:
>> From: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>>
>> Before each invocation of vsnprintf(), do_test() memsets the entire
>> allocated buffer to a sentinel value. That buffer includes leading and
>> trailing padding which is never included in the buffer area handed to
>> vsnprintf (spaces merely for clarity):
>>
>>   pad  test_buffer      pad
>>   **** **************** ****
>>
>> Then vsnprintf() is invoked with a bufsize argument <=
>> BUF_SIZE. Suppose bufsize=10, then we'd have e.g.
>>
>>  |pad |   test_buffer    |pad |
>>   **** pizza0 **** ****** ****
>>  A    B      C    D           E
>>
>> where vsnprintf() was given the area from B to D.
>>
>> It is obviously a bug for vsnprintf to touch anything between A and B
>> or between D and E. The former is checked for as one would expect. But
>> for the latter, we are actually a little stricter in that we check the
>> area between C and E.
>>
>> Split that check in two, providing a clearer error message in case it
>> was a genuine buffer overrun and not merely a write within the
>> provided buffer, but after the end of the generated string.
>>
>> So far, no part of the vsnprintf() implementation has had any use for
>> using the whole buffer as scratch space, but it's not unreasonable to
>> allow that, as long as the result is properly nul-terminated and the
>> return value is the right one. However, it is somewhat unusual, and
>> most %<something> won't need this, so keep the [C,D] check, but make
>> it easy for a later patch to make that part opt-out for certain tests.
> 
> Excellent commit message.
> 
>> Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
>> Tested-by: Jia He <justin.he@arm.com>
>> Signed-off-by: Jia He <justin.he@arm.com>
> 
> Reviewed-by: Petr Mladek <pmladek@suse.com>

Hi Petr

It seems Justin's series got stalled, but I still think this patch makes
sense on its own (especially since another series in flight mucks about
in this area), so can you please pick it up directly?

The lore link for the above is
https://lore.kernel.org/lkml/20210615154952.2744-4-justin.he@arm.com/ ,
while my original submission is at
https://lore.kernel.org/lkml/20210615085044.1923788-1-linux@rasmusvillemoes.dk/
.

Rasmus