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

Cyril Hrubis
2024-10-04T09:18:46Z

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.

--
Cyril Hrubis
chrubis@suse.cz