Re: [PATCH] Revert "vvfat: fix ubsan issue in create_long_filename"

[Philippe Mathieu-Daudé]

On 29/12/24 22:12, Michael Tokarev wrote:
> This reverts commit 0cb3ff7c22671aa1e1e227318799ccf6762c3bea.
>
> The original code was right in that long name in LFN directory
> entry uses other parts of the entry for the name too, not just
> the original "name" field.  So it is wrong to limit the offset
> to be within the name field.  Some other mechanism is needed
> to fix the ubsan report and whole messy usage of bytes past the
> given field.

Reported-by: Volker Rümelin <vr_qemu@t-online.de>

> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>   block/vvfat.c | 4 ----
>   1 file changed, 4 deletions(-)
>
> diff --git a/block/vvfat.c b/block/vvfat.c
> index f2eafaa923..8ffe8b3b9b 100644
> --- a/block/vvfat.c
> +++ b/block/vvfat.c
> @@ -426,10 +426,6 @@ static direntry_t *create_long_filename(BDRVVVFATState *s, 
> const char *filename)
>           else if(offset<22) offset=14+offset-10;
>           else offset=28+offset-22;
>           entry=array_get(&(s->directory),s->directory.next-1-(i/26));
> -        /* ensure we don't write anything past entry->name */
> -        if (offset >= sizeof(entry->name)) {
> -            continue;
> -        }
>           if (i >= 2 * length + 2) {
>               entry->name[offset] = 0xff;
>           } else if (i % 2 == 0) {