Re: [PATCH 3 of 3 RESEND] libxl: Warn that /usr/bin/pygrub is deprecated

[George Dunlap <george.dunlap@eu.citrix.com>]

On 10/05/12 12:44, Tim Deegan wrote:
> At 12:36 +0100 on 10 May (1336653395), Ian Jackson wrote:
>> George Dunlap writes ("Re: [Xen-devel] [PATCH 3 of 3 RESEND] libxl: Warn that /usr/bin/pygrub is deprecated"):
>>> On 09/05/12 14:43, Ian Campbell wrote:
>>>> On Wed, 2012-05-09 at 11:51 +0100, George Dunlap wrote:
>>>>> +    if ( !strncmp(info->u.pv.bootloader, "/usr/bin/pygrub", 20) )
>>>> Why strncmp and not just strcmp? And why 20? AFAIK
>>>> strlen("/usr/bin/pygrub") == 15 or 16 or so...
>>> ISTR in the past build processes throwing warnings that strcmp() is
>>> unsafe, and since warnings turn to errors, pre-emptively used the "safe"
>>> version instead.
>> Boggle.  Any such build processes need to be taken out and shot.
>> There is nothing wrong with strcmp.  Are you sure you're not thinking
>> of strcat or sprintf ?
> If the user controlled both the length and contents of
> info->u.pv.bootloader, it could cause this to overrun that buffer and
> cause a SEGV.  So, sadly, strcmp goes on the 'just never use it' list
> for many people.
Hmm, yes, I suppose it's *technically* possible that even when comparing 
to a static string, if info->u.pv.bootloader contains a short, 
non-null-terminated string, and were close to the edge of a page, it 
could cause a SEGV.  But using strncmp wouldn't solve that, would it?

  -George