From mboxrd@z Thu Jan 1 00:00:00 1970 From: Neil Brown Subject: Re: [PATCH 11/11] nfs-utils: mount: Fixed collision between commas in options Date: Thu, 1 Mar 2007 10:33:53 +1100 Message-ID: <17894.4449.797539.255376@notabene.brown> References: <45E2C289.30506@RedHat.com> <17891.53582.325117.716540@notabene.brown> <20070227093424.GE3954@petra.dvoda.cz> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Cc: Cory Olmo , nfs@lists.sourceforge.net, Steve Dickson To: Karel Zak Return-path: Received: from sc8-sf-mx1-b.sourceforge.net ([10.3.1.91] helo=mail.sourceforge.net) by sc8-sf-list2-new.sourceforge.net with esmtp (Exim 4.43) id 1HMYKB-0002Zo-Bj for nfs@lists.sourceforge.net; Wed, 28 Feb 2007 15:35:08 -0800 Received: from mx2.suse.de ([195.135.220.15]) by mail.sourceforge.net with esmtp (Exim 4.44) id 1HMYKA-0004Rs-Tp for nfs@lists.sourceforge.net; Wed, 28 Feb 2007 15:35:09 -0800 In-Reply-To: message from Karel Zak on Tuesday February 27 List-Id: "Discussion of NFS under Linux development, interoperability, and testing." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: nfs-bounces@lists.sourceforge.net Errors-To: nfs-bounces@lists.sourceforge.net On Tuesday February 27, kzak@redhat.com wrote: > On Tue, Feb 27, 2007 at 05:35:58PM +1100, Neil Brown wrote: > > On Monday February 26, SteveD@redhat.com wrote: > > > commit 96a3ceb3d35bf5edcb9446aded8375d3b98b4f5b > > > Author: Cory Olmo > > > Date: Sat Feb 24 16:20:40 2007 -0500 > > > > > > This patch avoid the collision between commas in security contexts and the > > > delimiter betweeen mount options. > > > > > > Signed-off-by: Karel Zak > > > Signed-off-by: Cory Olmo > > > > > > diff --git a/utils/mount/mount.c b/utils/mount/mount.c > > > index b3d3696..f22747b 100644 > > > --- a/utils/mount/mount.c > > > +++ b/utils/mount/mount.c > > > @@ -285,18 +285,30 @@ static void parse_opts (const char *options, int *flags, char **extra_opts) > > > { > > > if (options != NULL) { > > > char *opts = xstrdup(options); > > > - char *opt; > > > - int len = strlen(opts) + 20; > > > - > > > + char *opt, *p; > > > + int len = strlen(opts) + 256; > > > > This is a worry. If 20 isn't big enough, why do you thing 256 will > > The 20 was big enough for conversion from usernames to UIDs, but it's > not enough for selinux context names. I didn't found any definition > for maximal length of selinux context name, so we need to select any > range and assume that it's enough (or completely rewrite this part of > the mount command and allocate all dynamically). > > > be? and where do we check that the buffer doesn't overflow (in this > > setuid program). > > Please, read the code. We check the buffer size everywhere. I don't > see there any place where the buffer can overflow. OK, so I did read the code a bit more carefully, and now I'm even more confused and concerned. Firstly, I didn't notice that 'len' was being passed into parse_opt. However the way it is being passed makes it totally useless. parse_opt is called in a loop from parse_opts and each time it reduces 'len' by the amount that is added to 'extra_opts'. However 'len' is passed by value, not by reference, so each time around it starts at the same value and so is not effective in making sure that extra_opts doesn't overflow it's buffer. But further, I cannot see how extra_opts ever needs to be large than 'options'. All that is ever appended to it are sections that have been removed from 'options'. So why was the +20 there in the first place. You mention "conversion from usernames to UIDs", but I cannot find that anywhere. Could you please help me understand exactly why "extra_opts" needs to be larger than "options" ?? Thanks, NeilBrown ------------------------------------------------------------------------- Take Surveys. Earn Cash. Influence the Future of IT Join SourceForge.net's Techsay panel and you'll get the chance to share your opinions on IT & business topics through brief surveys-and earn cash http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV _______________________________________________ NFS maillist - NFS@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs