Re: [PATCH] audit: Fix check of return value of strnlen_user()

[Jan Kara <jack@suse.cz>]

On Wed 03-06-15 14:56:18, Paul Moore wrote:
> On Tuesday, June 02, 2015 05:08:29 PM Jan Kara wrote:
> > strnlen_user() returns 0 when it hits fault, not -1. Fix the test in
> > audit_log_single_execve_arg(). Luckily this shouldn't ever happen unless
> > there's a kernel bug so it's mostly a cosmetic fix.
> > 
> > CC: Paul Moore <pmoore@redhat.com>
> > Signed-off-by: Jan Kara <jack@suse.cz>
> > ---
> >  kernel/auditsc.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> > index 9fb9d1cb83ce..bb947ceeee4d 100644
> > --- a/kernel/auditsc.c
> > +++ b/kernel/auditsc.c
> > @@ -1023,7 +1023,7 @@ static int audit_log_single_execve_arg(struct
> > audit_context *context, * for strings that are too long, we should not have
> > created
> >  	 * any.
> >  	 */
> > -	if (unlikely((len == -1) || len > MAX_ARG_STRLEN - 1)) {
> > +	if (unlikely((len == 0) || len > MAX_ARG_STRLEN - 1)) {
> 
> While we're at it, should we make it just "len > MAX_ARG_STRLEN" as well?  
> Reading the comments in include/uapi/linux/binfmts.h as well as 
> valid_arg_len() that seems to be the correct logic.
  Umm, but audit_log_single_execve_arg() does decrement 1 from
strnlen_user() result before doing the comparison. So the current test
seems to match the one in valid_arg_len() exactly...

								Honza

-- 
Jan Kara <jack@suse.cz>
SUSE Labs, CR