Re: [LTP] [PATCH v4 13/13] Add statmount07 test

Jan Kara
2024-10-04T12:03:06Z

Hi!

On Fri 04-10-24 11:17:21, Cyril Hrubis wrote:
> > That seems to be a valid value, since I get PASS. Kernel bug?
>
> Looking at kernel it's a bit confusing, the bufsize is supposed to be
> the size of the structure plus the buffer allocated for the string
> reply. Which makes the code more complicated than it could have been if
> these two were separeted properly.
>
>
> However we do have:
>
> if (kbufsize >= s->bufsize)
> return -EOVERFLOW;
>
> in statmount_string(), so we will trigger the error there if we pass one
> of the requests that is supposed to produce a string reply.

Yes, if STATMOUNT_STRING_REQ is set, the above check should always make
sure bufsize is large enough.

> But there seems to be something strange going on in the
> prepare_kstatmount() as well, we do have:
>
> if (ks->mask & STATMOUNT_STRING_REQ) {
> if (bufsize == sizeof(ks->sm))
> return -EOVERFLOW;
>
> ^ This line should probably be if (bufsize <= sizeof(ks->sm))
> because we are trying to make sure that the structure is large
> enough that we can write at the offset where the
> string area starts.
>
> It seems to be guarded by the checks in
> statmount_string() later on as well, so I suppose that we
> will trigger the EOVERFLOW slightly later if this does not
> work though. CCying Jan to have a look.

Yup, this looks confusing to me as well. Christian, is this check indeed
meant to bail early if the buffer obviously is not large enough? Shouldn't
it then be <= as Cyril suggests? I have a feeling I might be missing some
"extensible syscall" magic that does the check bufsize < sizeof(struct)
automatically somewhere but I could not find it...

Honza
--
Jan Kara <jack@suse.com>
SUSE Labs, CR