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

Andrea Cervesato via ltp
2024-10-04T11:04:14Z

Thanks for the explanation! I will fix the other patches and send the
new version.

Andrea

On 10/4/24 11:17, Cyril Hrubis wrote:
> Hi!
>> 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.
>
>
> 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.
>
> ks->seq.buf = kvmalloc(seq_size, GFP_KERNEL_ACCOUNT);
> if (!ks->seq.buf)
> return -ENOMEM;
>
> ks->seq.size = seq_size;
> }
>
>
>
>
> However the size seems to be properly used in the
> copy_statmount_to_user()
>
> size_t copysize = min_t(size_t, s->bufsize, sizeof(*sm));
>
> ...
>
> if (copy_to_user(s->buf, sm, copysize))
> return -EFAULT;
>
>
> This means that we will copy as much of the statmount structure to
> userspace as we requested, so in the case that we pass 0 as bufsize none
> of the data should be copied and indeed we should get back success.
>