Re: [PATCH] rtld: Reject overly long LD_AUDIT path elements

[Carlos O'Donell <carlos at redhat dot com>]

On 06/29/2017 05:09 PM, Zack Weinberg wrote:
> On Thu, Jun 29, 2017 at 4:36 PM, Florian Weimer <fweimer@redhat.com> wrote:
>> On 06/29/2017 09:56 PM, Zack Weinberg wrote:
>>>
>>> I am not an experienced exploit writer either, and I don't know this
>>> code at all, but as a matter of principle, I do not think you should
>>> make any changes until Andreas actually explains his concerns in
>>> _detail_.  One-sentence cryptic questions, at a rate of one per email,
>>> are not proper code review.
>>
>> To be fair, the original patch went in without much review on
>> libc-alpha, too.
> 
> Carlos made clear that he had, in fact, read the entire thing, and I
> trust that if he had had concerns he would have said so.

I read the whole thing, and multiple times. I had no concerns with the 
patch. Is it exactly how I would have solved it? No. However, my 
responsibility as a reviewer is not to make everyone conform to the 
way *I* would have done it. In fact I appreciate immensely that Florian
comes up with answers that I didn't think about, because it adds
resilience to our community.

If there is a defect in the original patch I didn't detect it in my
review, and I reviewed over a dozen backports of the same patch. I think
I could rewrite it by hand without looking at the original.

The only overarching concern for *all* the patches is that we previously
had no limits, which is what the CVE is about, and now we *do* have limits
and that's because we need them to protect against this attack vector.

Eventually I forsee with -fstack-check we might lift the limits again,
but until then we need to limit exactly what secure applications can do.

-- 
Cheers,
Carlos.