Page MenuHomeFreeBSD
Differential D17282 Diff 61285 sys/compat/freebsd32/freebsd32_misc.c

Changeset View

Standalone View

View Options

sys/compat/freebsd32/freebsd32_misc.c

Show First 20 Lines • Show All 2,250 Lines • ▼ Show 20 Linesfreebsd32___sysctl(struct thread *td, struct freebsd32___sysctl_args *uap)
if (error) if (error)
return (error); return (error);
if (uap->oldlenp) if (uap->oldlenp)
suword32(uap->oldlenp, j); suword32(uap->oldlenp, j);
return (0); return (0);
} }
int int
freebsd32___sysctlbyname(struct thread *td,
struct freebsd32___sysctlbyname_args *uap)
{
size_t oldlen, rv;
int error = 0;
uint32_t tmp;
if (uap->oldlenp != NULL) {
error = fueword32(uap->oldlenp, &tmp);
oldlen = tmp;
} else {
mjgUnsubmitted
Done
Inline Actions

namelen is unsigned, so this should just check for == 0 instead of < 1

mjg: namelen is unsigned, so this should just check for == 0 instead of < 1
oldlen = 0;
kibUnsubmitted
Not Done
Inline Actions

Set error to zero in this branch, avoid initialization at declaration.

kib: Set error to zero in this branch, avoid initialization at declaration.
}
if (error != 0)
mjgUnsubmitted
Not Done
Inline Actions

vast majority of names are short, afair < 16. this should have a buffer of similar size to avoid having to do malloc in the common case.

mjg: vast majority of names are short, afair < 16. this should have a buffer of similar size to…
return (EFAULT);
error = kern___sysctlbyname(td, uap->name, uap->namelen, uap->old,
&oldlen, uap->new, uap->newlen, &rv, SCTL_MASK32, 1);
if (error != 0)
return (error);
if (uap->oldlenp != NULL)
mjgUnsubmitted
Not Done
Inline Actions

why the magic numbers

mjg: why the magic numbers
suword32(uap->oldlenp, rv);
kibUnsubmitted
Not Done
Inline Actions

error from suword32 should be preserved and returned there, I think.

kib: error from suword32 should be preserved and returned there, I think.
kaktusAuthorUnsubmitted
Done
Inline Actions

All other invocations of this function in this file don't do that either, and I don't know if returning EFAULT here when the action was already performed if the request was to change setting makes any sense.

kaktus: All other invocations of this function in this file don't do that either, and I don't know if…
return (0);
}
int
freebsd32_jail(struct thread *td, struct freebsd32_jail_args *uap) freebsd32_jail(struct thread *td, struct freebsd32_jail_args *uap)
{ {
kibUnsubmitted
Not Done
Inline Actions

if (error != 0)

kib: if (error != 0)
uint32_t version; uint32_t version;
int error; int error;
struct jail j; struct jail j;
kibUnsubmitted
Not Done
Inline Actions

if (oldlenp != NULL)

kib: if (oldlenp != NULL)
error = copyin(uap->jail, &version, sizeof(uint32_t)); error = copyin(uap->jail, &version, sizeof(uint32_t));
if (error) if (error)
return (error); return (error);
mjgUnsubmitted
Not Done
Inline Actions

the entire point of a __sysctlbyname syscall is to do all the sysctl work *once*. that is, the code should be reorganized so that the handler can be looked up by the name and then executed. this patch, just like the current approach, first finds the oid number and then performs yet another lookup to translate it the handler. this basically finds the handler twice. it does provide an upshot of only one syscall trip, but I don't think that's good enough of an improvement to integrate.

mjg: the entire point of a __sysctlbyname syscall is to do all the sysctl work *once*. that is, the…
kaktusAuthorUnsubmitted
Done
Inline Actions

OK, i'll rewrite this.

kaktus: OK, i'll rewrite this.
switch (version) { switch (version) {
case 0: case 0:
{ {
/* FreeBSD single IPv4 jails. */ /* FreeBSD single IPv4 jails. */
mjgUnsubmitted
Not Done
Inline Actions

this ignored the error

mjg: this ignored the error
kaktusAuthorUnsubmitted
Done
Inline Actions

So did every other usage of this function in this file, and in most cases in the whole kernel. I don't know if assigning EFAULT now, when the action was potentially made if this was write/set and not read only sysctl makes any sense here.

kaktus: So did every other usage of this function in this file, and in most cases in the whole kernel.
struct jail32_v0 j32_v0; struct jail32_v0 j32_v0;
bzero(&j, sizeof(struct jail)); bzero(&j, sizeof(struct jail));
error = copyin(uap->jail, &j32_v0, sizeof(struct jail32_v0)); error = copyin(uap->jail, &j32_v0, sizeof(struct jail32_v0));
if (error) if (error)
return (error); return (error);
CP(j32_v0, j, version); CP(j32_v0, j, version);
PTRIN_CP(j32_v0, j, path); PTRIN_CP(j32_v0, j, path);
▲ Show 20 Lines • Show All 1,207 Lines • Show Last 20 Lines