Skip to content

Commit 424a55a

Browse files
cypharbrauner
authored andcommitted
uaccess: add copy_struct_to_user helper
This is based on copy_struct_from_user(), but there is one additional case to consider when creating a syscall that returns an extensible-struct to userspace -- how should data in the struct that cannot fit into the userspace struct be handled (ksize > usize)? There are three possibilies: 1. The interface is like sched_getattr(2), where new information will be silently not provided to userspace. This is probably what most interfaces will want to do, as it provides the most possible backwards-compatibility. 2. The interface is like lsm_list_modules(2), where you want to return an error like -EMSGSIZE if not providing information could result in the userspace program making a serious mistake (such as one that could lead to a security problem) or if you want to provide some flag to userspace so they know that they are missing some information. 3. The interface is like statx(2), where there some kind of a request mask that indicates what data userspace would like. One could imagine that statx2(2) (using extensible structs) would want to return -EMSGSIZE if the user explicitly requested a field that their structure is too small to fit, but not return an error if the field was not explicitly requested. This is kind of a mix between (1) and (2) based on the requested mask. The copy_struct_to_user() helper includes a an extra argument that is used to return a boolean flag indicating whether there was a non-zero byte in the trailing bytes that were not copied to userspace. This can be used in the following ways to handle all three cases, respectively: 1. Just pass NULL, as you don't care about this case. 2. Return an error (say -EMSGSIZE) if the argument was set to true by copy_struct_to_user(). 3. If the argument was set to true by copy_struct_to_user(), check if there is a flag that implies a field larger than usize. This is the only case where callers of copy_struct_to_user() should check usize themselves. This will probably require scanning an array that specifies what flags were added for each version of the flags struct and returning an error if the request mask matches any of the flags that were added in versions of the struct that are larger than usize. At the moment we don't have any users of (3), so this patch doesn't include any helpers to make the necessary scanning easier, but it should be fairly easy to add some if necessary. Signed-off-by: Aleksa Sarai <[email protected]> Link: https://lore.kernel.org/r/20241010-extensible-structs-check_fields-v3-1-d2833dfe6edd@cyphar.com Signed-off-by: Christian Brauner <[email protected]>
1 parent 8e929cb commit 424a55a

File tree

1 file changed

+97
-0
lines changed

1 file changed

+97
-0
lines changed

include/linux/uaccess.h

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,103 @@ copy_struct_from_user(void *dst, size_t ksize, const void __user *src,
394394
return 0;
395395
}
396396

397+
/**
398+
* copy_struct_to_user: copy a struct to userspace
399+
* @dst: Destination address, in userspace. This buffer must be @ksize
400+
* bytes long.
401+
* @usize: (Alleged) size of @dst struct.
402+
* @src: Source address, in kernel space.
403+
* @ksize: Size of @src struct.
404+
* @ignored_trailing: Set to %true if there was a non-zero byte in @src that
405+
* userspace cannot see because they are using an smaller struct.
406+
*
407+
* Copies a struct from kernel space to userspace, in a way that guarantees
408+
* backwards-compatibility for struct syscall arguments (as long as future
409+
* struct extensions are made such that all new fields are *appended* to the
410+
* old struct, and zeroed-out new fields have the same meaning as the old
411+
* struct).
412+
*
413+
* Some syscalls may wish to make sure that userspace knows about everything in
414+
* the struct, and if there is a non-zero value that userspce doesn't know
415+
* about, they want to return an error (such as -EMSGSIZE) or have some other
416+
* fallback (such as adding a "you're missing some information" flag). If
417+
* @ignored_trailing is non-%NULL, it will be set to %true if there was a
418+
* non-zero byte that could not be copied to userspace (ie. was past @usize).
419+
*
420+
* While unconditionally returning an error in this case is the simplest
421+
* solution, for maximum backward compatibility you should try to only return
422+
* -EMSGSIZE if the user explicitly requested the data that couldn't be copied.
423+
* Note that structure sizes can change due to header changes and simple
424+
* recompilations without code changes(!), so if you care about
425+
* @ignored_trailing you probably want to make sure that any new field data is
426+
* associated with a flag. Otherwise you might assume that a program knows
427+
* about data it does not.
428+
*
429+
* @ksize is just sizeof(*src), and @usize should've been passed by userspace.
430+
* The recommended usage is something like the following:
431+
*
432+
* SYSCALL_DEFINE2(foobar, struct foo __user *, uarg, size_t, usize)
433+
* {
434+
* int err;
435+
* bool ignored_trailing;
436+
* struct foo karg = {};
437+
*
438+
* if (usize > PAGE_SIZE)
439+
* return -E2BIG;
440+
* if (usize < FOO_SIZE_VER0)
441+
* return -EINVAL;
442+
*
443+
* // ... modify karg somehow ...
444+
*
445+
* err = copy_struct_to_user(uarg, usize, &karg, sizeof(karg),
446+
* &ignored_trailing);
447+
* if (err)
448+
* return err;
449+
* if (ignored_trailing)
450+
* return -EMSGSIZE:
451+
*
452+
* // ...
453+
* }
454+
*
455+
* There are three cases to consider:
456+
* * If @usize == @ksize, then it's copied verbatim.
457+
* * If @usize < @ksize, then the kernel is trying to pass userspace a newer
458+
* struct than it supports. Thus we only copy the interoperable portions
459+
* (@usize) and ignore the rest (but @ignored_trailing is set to %true if
460+
* any of the trailing (@ksize - @usize) bytes are non-zero).
461+
* * If @usize > @ksize, then the kernel is trying to pass userspace an older
462+
* struct than userspace supports. In order to make sure the
463+
* unknown-to-the-kernel fields don't contain garbage values, we zero the
464+
* trailing (@usize - @ksize) bytes.
465+
*
466+
* Returns (in all cases, some data may have been copied):
467+
* * -EFAULT: access to userspace failed.
468+
*/
469+
static __always_inline __must_check int
470+
copy_struct_to_user(void __user *dst, size_t usize, const void *src,
471+
size_t ksize, bool *ignored_trailing)
472+
{
473+
size_t size = min(ksize, usize);
474+
size_t rest = max(ksize, usize) - size;
475+
476+
/* Double check if ksize is larger than a known object size. */
477+
if (WARN_ON_ONCE(ksize > __builtin_object_size(src, 1)))
478+
return -E2BIG;
479+
480+
/* Deal with trailing bytes. */
481+
if (usize > ksize) {
482+
if (clear_user(dst + size, rest))
483+
return -EFAULT;
484+
}
485+
if (ignored_trailing)
486+
*ignored_trailing = ksize < usize &&
487+
memchr_inv(src + size, 0, rest) != NULL;
488+
/* Copy the interoperable parts of the struct. */
489+
if (copy_to_user(dst, src, size))
490+
return -EFAULT;
491+
return 0;
492+
}
493+
397494
bool copy_from_kernel_nofault_allowed(const void *unsafe_src, size_t size);
398495

399496
long copy_from_kernel_nofault(void *dst, const void *src, size_t size);

0 commit comments

Comments
 (0)