[PATCH] squash: review-updates

William Roberts bill.c.roberts at gmail.com
Fri Jan 10 21:42:09 UTC 2014


hmm if I recall too the result from calling the underlying vma
functions was the width of the field, not
always necessarily the null byte, like you said, doing the combo you
suggest corrects that.

However, strnlen is pricey id like to avoid. The code I have at least
avoids branching, which is good.
And is then directly known to be safe to pass to
audit_log_untrusted_string(), which aborts printing
on the first null byte.

On Fri, Jan 10, 2014 at 1:37 PM, William Roberts
<bill.c.roberts at gmail.com> wrote:
> I think your right....
>
> On Fri, Jan 10, 2014 at 1:08 PM, Eric Paris <eparis at redhat.com> wrote:
>> If you know the buf len, you can just use audit_log_n_untrusted_string()
>> I think....
>>
>> On Tue, 2014-01-07 at 12:44 -0800, William Roberts wrote:
>>> Signed-off-by: William Roberts <wroberts at tresys.com>
>>> ---
>>>  kernel/auditsc.c |   19 +++++++++++++++----
>>>  1 file changed, 15 insertions(+), 4 deletions(-)
>>>
>>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>>> index a4c2003..9ba1f2a 100644
>>> --- a/kernel/auditsc.c
>>> +++ b/kernel/auditsc.c
>>> @@ -1292,9 +1292,20 @@ static void audit_log_cmdline(struct audit_buffer *ab, struct task_struct *tsk,
>>>               if (!buf)
>>>                       goto out;
>>>               res = get_cmdline(tsk, buf, PATH_MAX);
>>> -             /* Ensure NULL terminated */
>>> -             if (buf[res-1] != '\0')
>>> -                     buf[res-1] = '\0';
>>> +             if (res == 0) {
>>> +                     kfree(buf);
>>> +                     goto out;
>>> +             }
>>> +             /*
>>> +              * Ensure NULL terminated but don't clobber the end
>>> +              * unless the buffer is full. Worst case you end up
>>> +              * with 2 null bytes ending it. By doing it this way
>>> +              * one avoids additional branching. One checking if the
>>> +              * end is null and another to check if their should be
>>> +              * an increment before setting the null byte.
>>> +              */
>>> +             res += res < PATH_MAX;
>>> +             buf[res-1] = '\0';
>>>               context->cmdline = buf;
>>>       }
>>>       msg = context->cmdline;
>>> @@ -1333,8 +1344,8 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *ts
>>>                        context->name_count);
>>>
>>>       audit_log_task_info(ab, tsk);
>>> -     audit_log_cmdline(ab, tsk, context);
>>>       audit_log_key(ab, context->filterkey);
>>> +     audit_log_cmdline(ab, tsk, context);
>>>       audit_log_end(ab);
>>>
>>>       for (aux = context->aux; aux; aux = aux->next) {
>>
>>
>
>
>
> --
> Respectfully,
>
> William C Roberts



-- 
Respectfully,

William C Roberts




More information about the Linux-audit mailing list