Quoting Stephen Smalley ([email protected]):
> On Fri, 2006-06-30 at 14:37 -0500, Serge E. Hallyn wrote:
> > Quoting James Morris ([email protected]):
> > ...
> > > +static int get_task_ioprio(struct task_struct *p)
> > > +{
> > > + int ret;
> > > +
> > > + ret = security_task_getioprio(p);
> > > + if (ret)
> > > + goto out;
> > > + ret = p->ioprio;
> > > +out:
> > > + return ret;
> > > +}
> > ...
> > > do_each_task_pid(who, PIDTYPE_PGID, p) {
> > > + tmpio = get_task_ioprio(p);
> > > + if (tmpio < 0)
> > > + continue;
> > > if (ret == -ESRCH)
> > > - ret = p->ioprio;
> > > + ret = tmpio;
> > > else
> > > - ret = ioprio_best(ret, p->ioprio);
> > > + ret = ioprio_best(ret, tmpio);
> > ...
> > > + * @task_getioprio
> > > + * Check permission before getting the ioprio value of @p.
> > > + * @p contains the task_struct of process.
> > > + * Return 0 if permission is granted.
> >
> > A return value >0 is a problem here but isn't mentioned. the
> > get_task_ioprio() helper will return the the security_task_getioprio()
> > return value in htat case, but the do_each_task_pid loop will take it
> > as a valid return value.
>
> True, but that isn't limited to that hook - think what happens if
> inode_permission returns an arbitrary positive integer.
Well I don't know whether it's worth fixing for all of the lsm hooks,
maybe it is. But in this case in particular there is just no reason for
it, and the fix is trivial... Just switch the quoted parts to:
> +static int get_task_ioprio(struct task_struct *p)
> +{
> + int ret;
> +
> + ret = security_task_getioprio(p);
> + if (ret<0)
> + goto out;
> + ret = p->ioprio;
> +out:
> + return ret;
> +}
...
> do_each_task_pid(who, PIDTYPE_PGID, p) {
> + tmpio = get_task_ioprio(p);
> + if (tmpio < 0)
> + continue;
> if (ret == -ESRCH)
> - ret = p->ioprio;
> + ret = tmpio;
> else
> - ret = ioprio_best(ret, p->ioprio);
> + ret = ioprio_best(ret, tmpio);
...
> + * @task_getioprio
> + * Check permission before getting the ioprio value of @p.
> + * @p contains the task_struct of process.
> + * Return <0 if permission is denied, 0 if granted.
I'm not saying I'd expect this to be triggered, since most kernel and
lsm coders wouldn't think of returning >0 for error in any case... Yet
you never know. And again, there's just no reason for it.
-serge
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
