From mboxrd@z Thu Jan 1 00:00:00 1970 From: George Dunlap Subject: Re: [PATCH 3 of 3 RESEND] libxl: Warn that /usr/bin/pygrub is deprecated Date: Thu, 10 May 2012 12:47:24 +0100 Message-ID: <4FABAACC.9000301@eu.citrix.com> References: <794778a6e9fa761bd388.1336560666@kodo2> <1336570982.25514.120.camel@zakaz.uk.xensource.com> <4FAA83A8.8070804@eu.citrix.com> <20395.43075.534483.485017@mariner.uk.xensource.com> <20120510114414.GC73773@ocelot.phlegethon.org> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120510114414.GC73773@ocelot.phlegethon.org> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Tim Deegan Cc: "xen-devel@lists.xensource.com" , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org 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