Re: [Qemu-devel] git master build failure in 9pfs

[Greg Kurz]

On Fri, 3 Mar 2017 16:21:28 +0000
"Daniel P. Berrange" <address@hidden> wrote:

> On Fri, Mar 03, 2017 at 10:55:01AM -0500, G 3 wrote:
> > 
> > On Mar 3, 2017, at 10:44 AM, Greg Kurz wrote:
> >   
> > > On Fri, 3 Mar 2017 10:28:00 -0500
> > > G 3 <address@hidden> wrote:
> > >   
> > > > On Mar 3, 2017, at 9:59 AM, address@hidden wrote:  
> > > > > On 02/03/17 17:40, Daniel P. Berrange wrote:
> > > > >   
> > > > > > On Thu, Mar 02, 2017 at 05:28:24PM +0000, Mark Cave-Ayland wrote:  
> > > > > > > Does anyone else see the following error when trying to build git
> > > > > > > master?
> > > > > > > 
> > > > > > > cc -I/home/build/src/qemu/git/qemu/hw/9pfs -Ihw/9pfs
> > > > > > > -I/home/build/src/qemu/git/qemu/tcg
> > > > > > > -I/home/build/src/qemu/git/qemu/tcg/i386
> > > > > > > -I/home/build/src/qemu/git/qemu/linux-headers
> > > > > > > -I/home/build/src/qemu/git/qemu/linux-headers -I.
> > > > > > > -I/home/build/src/qemu/git/qemu -I/home/build/src/qemu/git/qemu/
> > > > > > > include
> > > > > > > -I/usr/include/pixman-1
> > > > > > > -I/home/build/src/qemu/git/qemu/dtc/libfdt
> > > > > > > -Werror -pthread -I/usr/include/glib-2.0
> > > > > > > -I/usr/lib/x86_64-linux-gnu/glib-2.0/include   -m64 -mcx16 -
> > > > > > > D_GNU_SOURCE
> > > > > > > -D_FILE_OFFSET_BITS=64 -D_LARGEFILE_SOURCE -Wstrict-prototypes
> > > > > > > -Wredundant-decls -Wall -Wundef -Wwrite-strings
> > > > > > > -Wmissing-prototypes
> > > > > > > -fno-strict-aliasing -fno-common -fwrapv  -Wendif-labels
> > > > > > > -Wno-missing-include-dirs -Wempty-body -Wnested-externs
> > > > > > > -Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers
> > > > > > > -Wold-style-declaration -Wold-style-definition -Wtype-limits
> > > > > > > -fstack-protector-all -I/usr/include/p11-kit-1
> > > > > > > -I/usr/include/libpng12   -I/home/build/src/qemu/git/qemu/tests -
> > > > > > > MMD -MP
> > > > > > > -MT hw/9pfs/9p-util.o -MF hw/9pfs/9p-util.d -O2 -U_FORTIFY_SOURCE
> > > > > > > -D_FORTIFY_SOURCE=2 -g   -c -o hw/9pfs/9p-util.o hw/9pfs/9p-util.c
> > > > > > > In file included from hw/9pfs/9p-util.c:15:0:
> > > > > > > hw/9pfs/9p-util.h: In function ?openat_dir?:
> > > > > > > hw/9pfs/9p-util.h:25:57: error: ?O_PATH? undeclared (first use in
> > > > > > > this
> > > > > > > function)
> > > > > > > hw/9pfs/9p-util.h:25:57: note: each undeclared identifier is
> > > > > > > reported
> > > > > > > only once for each function it appears in
> > > > > > > hw/9pfs/9p-util.h:26:1: error: control reaches end of non-void
> > > > > > > function
> > > > > > > [-Werror=return-type]
> > > > > > > 
> > > > > > > Build platform is Debian Wheezy on an x86_64 host.  
> > > > > > 
> > > > > > IIUC, O_PATH was introduced in glibc 2.14 and Wheezy only has 2.13.
> > > > > > 
> > > > > > So unless we want to make this 9pfs code a configurable
> > > > > > option, this
> > > > > > means Debian Wheezy is no longer a supportable platform for QEMU.  
> > > > > 
> > > > > Oh sure, I appreciate that wheezy is getting towards then end of its
> > > > > lifetime - it's just a little bit inconvenient to break my
> > > > > development
> > > > > environment just as we enter 2.9 freeze ;)
> > > > > 
> > > > > If everyone agrees that wheezy is no longer supported after 2.9
> > > > > then I
> > > > > can look to upgrading, however my QEMU development is done on my
> > > > > laptop
> > > > > which is also setup for my day job so it's not a simple case of just
> > > > > switching the repository and running dist-upgrade to get me going
> > > > > again...  
> > > > 
> > > > I remember years ago something like O_PATH was not defined on Mac OS
> > > > X,
> > > > so the solution was to define the constant as zero. Something like
> > > > this:
> > > > 
> > > > #ifndef O_PATH
> > > >         #define O_PATH 0
> > > > #endif
> > > > 
> > > > Maybe this might work in 9p-util.h.
> > > >   
> > > 
> > > Yes. Please send a patch and I'll merge it.
> > > 
> > > Cheers.
> > > 
> > > --
> > > Greg  
> > 
> > 
> > Here is the patch. I think we should let Mark or some else test it to see if
> > it does fix the problem before a real patch is submitted.
> > 
> > ---
> >  hw/9pfs/9p-util.h | 4 ++++
> >  1 file changed, 4 insertions(+)
> > 
> > diff --git a/hw/9pfs/9p-util.h b/hw/9pfs/9p-util.h
> > index 091f3ce..254d2a9 100644
> > --- a/hw/9pfs/9p-util.h
> > +++ b/hw/9pfs/9p-util.h
> > @@ -13,6 +13,10 @@
> >  #ifndef QEMU_9P_UTIL_H
> >  #define QEMU_9P_UTIL_H
> > 
> > +#ifndef O_PATH
> > +    #define O_PATH 0
> > +#endif  
> 
> Isn't the use of O_PATH required in order to fix the recent
> security vulnerability in 9p ?  If so, then defining it to
> 0 means the QEMU is silently becoming vulnerable once again
> which I don't think is a good idea.
> 

O_PATH was supposed to be used as an optimization here, since fds returned by
this function are only passed to openat()... but your comment makes me realize
I inadvertently dropped O_NOFOLLOW between v1 and v2 of the patchset. And this
IS an actual vulnerability issue :) And reading the openat() manpage, I see
that O_PATH | O_NOFOLLOW doesn't cause openat() to fail, but to return a fd
pointing to the symlink which is certainly not what I want :)

I guess I'll just stop using O_PATH then. I'll send a patch.

> 
> Regards,
> Daniel

pgpwPWGihUk3N.pgp
Description: OpenPGP digital signature