On Wed, Jun 19, 2019 at 10:31:45AM -0700, Casey Schaufler wrote:

On 6/18/2019 10:41 PM, Kees Cook wrote:

quoted

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. :)

quoted

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