On Wed, Nov 02, 2016 at 02:24:43PM -0500, Eric van Gyzen wrote:
> Does copyinstr guarantee that it has filled the output buffer when it
> returns ENAMETOOLONG?  I usually try to answer my own questions, but I
> don't speak many dialects of assembly.  :)

The few I checked appear to work by checking the length while copying,
but the man page copy(9) does not guarantee that, and similar code in
sys/compat/linux/linux_misc.c linux_prctl() LINUX_PR_SET_NAME performs a
copyin() if copyinstr() fails with [ENAMETOOLONG].

In implementing copyinstr(), checking the length first may make sense
for economy of code: a user-strnlen() using an algorithm like
lib/libc/string/strlen.c and a copyin() connected together with a C
copyinstr(). This would probably be faster than the current amd64 code
(which uses lods and stos, meh). With that implementation, filling the
buffer in the [ENAMETOOLONG] case requires a small piece of additional
code.

> I ask because I'd like to make the following change, and I'd like to
> know whether I should zero the buffer before calling copyinstr to ensure
> that I don't set the thread's name to the garbage that was on the stack.

> Index: kern_thr.c
> ===================================================================
> --- kern_thr.c        (revision 308217)
> +++ kern_thr.c        (working copy)
> @@ -580,8 +580,13 @@ sys_thr_set_name(struct thread *td, struct thr_set
>       if (uap->name != NULL) {
>               error = copyinstr(uap->name, name, sizeof(name),
>                       NULL);
> -             if (error)
> -                     return (error);
> +             if (error) {
> +                     if (error == ENAMETOOLONG) {
> +                             name[sizeof(name) - 1] = '\0';
> +                     } else {
> +                             return (error);
> +                     }
> +             }
>       }
>       p = td->td_proc;
>       ttd = tdfind((lwpid_t)uap->id, p->p_pid);

For API design, it makes more sense to set error = 0 if a truncated name
is being set anyway. This preserves the property that the name remains
unchanged if the call fails.

A change to the man page thr_set_name(2) is needed in any case.

-- 
Jilles Tjoelker
_______________________________________________
freebsd-current@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/freebsd-current
To unsubscribe, send any mail to "freebsd-current-unsubscr...@freebsd.org"

Reply via email to