Re: [PATCH] DoS in RPC implementation (CVE-2011-4069)

["Carlos O'Donell" <carlos at systemhalted dot org>]

On Wed, Nov 28, 2012 at 1:16 PM, Jeff Law <law@redhat.com> wrote:
> On 11/21/2012 03:10 PM, Roland McGrath wrote:
>>>
>>> --- a/sunrpc/Versions
>>> +++ b/sunrpc/Versions
>>> @@ -118,5 +118,6 @@ libc {
>>>     }
>>>     GLIBC_PRIVATE {
>>>       __libc_clntudp_bufcreate; __libc_rpc_getport;
>>> +    __svc_accept_failed;
>>
>> [...]
>>>
>>> --- a/sunrpc/rpc/svc.h
>>> +++ b/sunrpc/rpc/svc.h
>>> @@ -316,4 +316,5 @@ extern SVCXPRT *svcunix_create (int __sock, u_int
>>> __sendsize, u_int __recvsize,
>>>
>>>   __END_DECLS
>>>
>>> +extern void __svc_accept_failed (void) attribute_hidden;
>>
>>
>> Either it's hidden or it needs to be exported, not both.
>
> It should be hidden (I think); it doesn't need to be used by anything except
> the internal routines within the sunrpc code.
>
> Here's the updated version with the patch to sunrpc/Versions eliminated.

Please inline your patches (see the contribution checklist please) it
makes it much easier to carry out the reply/review cycle :-)

Your patch looks good, I see one issues though:

* Legally significant changes.

Your changes are legally significant IMO, and therefore should have a
new copyright holder added to all files for the changes you made.

FSF legal has said (thanks Pedro for tracking this down):
~~~
1) It is certainly the case that you should add a copyright line for the
FSF when nontrivial changes are made.  This is what reflects reality,
and it has always been the stated practice to follow, though I gather
libc did not bother to do so.  It is explicitly discussed at the end of
the Copyright Notices section you cited,
http://www.gnu.org/prep/maintain/maintain.html#Copyright-Notices.
~~~

Which is the reason for the update in the contribution checklist here:
http://sourceware.org/glibc/wiki/Contribution%20checklist#Update_Copyright_Information

Please add a new copyright notice to the files in addition to the oracle one.

Cheers,
Carlos.