On Wed, Jun 19, 2019 at 10:31:45AM -0700, Casey Schaufler wrote: > On 6/18/2019 10:41 PM, Kees Cook wrote: > > I think this is wrong: for NUL-terminated strings, "context.len" isn't > > currently including the NUL byte (it's set to strlen()). > > > > So, if kmemdup() is used here, it means strlen() isn't correct in the > > context init helper, it should be using the "size" argument, etc. > > Would all be true if the context where being set by lsmcontext_init, > but it's not. It's coming from the dentry_init_security hook, and > the one instance of that, selinux_dentry_init_security() sets the > size to strlen() + 1. The kmemdup() will include the terminating NUL. Ah-ha! Okay, thanks, yes. I see now. Carry on! :) > I too wish that the hooks and their use where more consistent. > My sincere hope is that this revision of the infrastructure will > help that to some extent. Once these changes land it should be much much easier to find ways to refactor for greater sanity. :) > > Should label be set to NULL here and len reduced to 0? > > It wasn't before, and I'd hate to make too many assumptions > about what might be fragile in the NFS code. Gotcha. -- Kees Cook
