tech-kern archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
Re: Fix netbsd32's getfh()
In article <53AA91A3.1070300%M00nBSD.net@localhost>,
Maxime Villard <max%M00nBSD.net@localhost> wrote:
>Hi,
>Here is a patch to sync netbsd32 with the native getfh() syscall. In addition
>to making it consistent, it also:
>
>a) fixes the return value:
> } else if (error == E2BIG) {
> error = copyout(&sz, SCARG_P32(uap, fh_size),
> sizeof(size_t));
> }
> here the error code is overwritten by copyout(), so it won't ever return
> E2BIG
>
>b) fixes a leak:
> if (fh == NULL)
> return EINVAL;
> a vput(vp) is missing here
>
>c) fixes a user-controlled allocation:
> fh = kmem_alloc(sz32, KM_SLEEP);
>
>I would like some ok's before committing it. Tested on NetBSD-current/amd64.
>
>Thanks.
>
>
>Index: netbsd32_netbsd.c
>===================================================================
>RCS file: /cvsroot/src/sys/compat/netbsd32/netbsd32_netbsd.c,v
>retrieving revision 1.190
>diff -u -r1.190 netbsd32_netbsd.c
>--- netbsd32_netbsd.c 22 Jun 2014 19:09:39 -0000 1.190
>+++ netbsd32_netbsd.c 25 Jun 2014 07:21:23 -0000
>@@ -1302,7 +1302,7 @@
> int error;
> struct pathbuf *pb;
> struct nameidata nd;
>- netbsd32_size_t sz32;
>+ netbsd32_size_t usz32, sz32;
> size_t sz;
>
> /*
>@@ -1312,7 +1312,6 @@
> 0, NULL, NULL, NULL);
> if (error)
> return (error);
>- fh = NULL;
>
> error = pathbuf_copyin(SCARG_P32(uap, fname), &pb);
> if (error) {
>@@ -1328,30 +1327,31 @@
> vp = nd.ni_vp;
> pathbuf_destroy(pb);
>
>- error = copyin(SCARG_P32(uap, fh_size), &sz32,
>+ error = vfs_composefh_alloc(vp, &fh);
>+ vput(vp);
>+ if (error != 0) {
>+ goto out;
That should probably be:
return error;
since vfs_composefh_alloc() failed, and we should not be calling
vfs_composefh_free().
>+ }
>+ error = copyin(SCARG_P32(uap, fh_size), &usz32,
> sizeof(netbsd32_size_t));
I would change sizeof(netbsd32_size_t) sizeof(usz32), that makes it
fit on one line too :-)
>- if (error) {
>- vput(vp);
>- return error;
>+ if (error != 0) {
>+ goto out;
> }
>- fh = kmem_alloc(sz32, KM_SLEEP);
>- if (fh == NULL)
>- return EINVAL;
>- sz = sz32;
>- error = vfs_composefh(vp, fh, &sz);
>- vput(vp);
>+ sz = FHANDLE_SIZE(fh);
>+ sz32 = sz;
>
>- if (error == 0) {
>- const netbsd32_size_t nsz32 = sz;
>- error = copyout(&nsz32, SCARG_P32(uap, fh_size),
>- sizeof(netbsd32_size_t));
>- if (!error) {
>- error = copyout(fh, SCARG_P32(uap, fhp), sz);
>- }
>- } else if (error == E2BIG) {
>- error = copyout(&sz, SCARG_P32(uap, fh_size), sizeof(size_t));
>+ error = copyout(&sz32, SCARG_P32(uap, fh_size),
>+ sizeof(netbsd32_size_t));
Again perhaps sizeof(sz32)...
christos
>+ if (error != 0) {
>+ goto out;
>+ }
>+ if (usz32 >= sz32) {
>+ error = copyout(fh, SCARG_P32(uap, fhp), sz);
>+ } else {
>+ error = E2BIG;
> }
>- kmem_free(fh, sz32);
>+out:
>+ vfs_composefh_free(fh);
> return (error);
> }
>
>
Home |
Main Index |
Thread Index |
Old Index