Re: [kernel-hardening] Re: [PATCH 1/2] sysctl: expand use of proc_dointvec_minmax_sysadmin

[Jann Horn]

On Sun, Jan 24, 2016 at 12:02:41AM -0600, Eric W. Biederman wrote:
> Jann Horn <jann@xxxxxxxxx> writes:
> 
> > On Sun, Jan 24, 2016 at 01:43:42AM +0000, Al Viro wrote:
> >> On Sat, Jan 23, 2016 at 07:20:17PM -0600, Eric W. Biederman wrote:
> >> 
> >> > Yep.  That is about the size of it.  file * used to be passed to the
> >> > sysctl methods but it was removed several years ago because no one was
> >> > using it.
> >> 
> >> Generally cred would be better...
> >
> >> Alternatively we could eat one more
> >> pointer in task_struct and stash a reference to that sucker there, rather
> >> than adding an explicit argument (again, with cred instead of file).
> >> Not sure...
> >
> > I think it makes sense to do this the same way as the rest of the VFS code
> > here (which passes the creds down through an argument).
> >
> > And adding the arguments everywhere doesn't really mean more work - either
> > way, someone should probably go through all of those sysctl handlers and
> > fix them up to use the file creds.
> 
> Not all of them need it.  It might be worth figuring out the necessary
> rigamarole to hook into sysctl_perm the way the networking code does and
> have that require the capability at open time.
> 
> The advantage is that open time is when it is actually appropraite to
> check permissions.  I could be wrong but I doubt there is enough madness
> with the handful of sysctl users that call capable to require the checks
> to happen on write and not on open.

That would work - if all sysctls know whether a capability will be needed
for writing later on and don't decide it based on the written data. Is that
always true?

Looking through some of the sysctl handlers, I found proc_do_uts_string and
pid_ns_ctl_handler, which operate on a namespace looked up through current
at write time. I think that's buggy and ought to be done using the file
opener creds and on the file opener's namespaces, but where can those be
stored?

Attachment: signature.asc
Description: Digital signature