linux-kernel - Re: [PATCH 10/10] NFSD: Server implementation of MAC Labeling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 7 Jul 2010 15:24:31 -0400
From:	"J. Bruce Fields" <bfields@...ldses.org>
To:	"David P. Quigley" <dpquigl@...ho.nsa.gov>
Cc:	hch@...radead.org, viro@...iv.linux.org.uk, casey@...aufler-ca.com,
	sds@...ho.nsa.gov, matthew.dodd@...rta.com,
	trond.myklebust@....uio.no, linux-kernel@...r.kernel.org,
	linux-fsdevel@...r.kernel.org,
	linux-security-module@...r.kernel.org, selinux@...ho.nsa.gov,
	linux-nfs@...r.kernel.org
Subject: Re: [PATCH 10/10] NFSD: Server implementation of MAC Labeling

On Wed, Jul 07, 2010 at 02:03:21PM -0400, David P. Quigley wrote:
> Comments inline
> 
> On Wed, 2010-07-07 at 13:21 -0400, J. Bruce Fields wrote:
> > On Wed, Jul 07, 2010 at 10:31:26AM -0400, David P. Quigley wrote:
> > > This patch adds the ability to encode and decode file labels on the server for
> > >  static __be32
> > >  nfsd4_decode_fattr(struct nfsd4_compoundargs *argp, u32 *bmval,
> > > -		   struct iattr *iattr, struct nfs4_acl **acl)
> > > +		   struct iattr *iattr, struct nfs4_acl **acl,
> > > +		   struct nfs4_label **label)
> > 
> > As we add more arguments, I wonder if at some point it becomes worth
> > creating something like
> > 
> > 	struct nfsd4_attrs {
> > 		struct iattr iattr;
> > 		struct nfs4_acl *acl;
> > 		...
> > 	}
> > 
> > and passing that instead?
> 
> I can definitely submit something like that as a stand alone patch and
> then add our nfs4_label stuff to that instead.

Not a big deal, but welcome if such a thing looks more generally useful
(say, if the same arguments are passed elsewhere).

> > > +#ifdef CONFIG_NFSD_V4_SECURITY_LABEL
> > > +	if (bmval[1] & FATTR4_WORD1_SECURITY_LABEL) {
> > > +		uint32_t lfs;
> > > +
> > > +		READ_BUF(4);
> > > +		len += 4;
> > > +		READ32(lfs);
> > > +		READ_BUF(4);
> > > +		len += 4;
> > > +		READ32(dummy32);
> > > +		READ_BUF(dummy32);
> > > +		len += (XDR_QUADLEN(dummy32) << 2);
> > > +		READMEM(buf, dummy32);
> > > +
> > > +		if (dummy32 > NFS4_MAXLABELLEN)
> > > +			return nfserr_resource;
> > > +
> > > +		*label = kzalloc(sizeof(struct nfs4_label), GFP_KERNEL);
> > 
> > Could we allocate this some toher way (it's small, right?) and avoid the
> > extra dynamic allocation here, just for simplicity's sake?
> 
> How would you suggest going about doing that?

Maybe make op_label, cr_label, etc., a struct nfs4_label instead of a
struct nfs4_label *?

> 
> > 
> > > +		if (*label == NULL) {
> > > +			host_err = -ENOMEM;
> > > +			goto out_nfserr;
> > > +		}
> > > +
> > > +		(*label)->label = kmalloc(dummy32 + 1, GFP_KERNEL);
> > 
> > Might be nice to arrange NFS4_MAXLABELLEN to ensure this is never a
> > higher-order allocation.
> 
> This should never be more than 4096. Under what circumstances would this
> become a higher-order allocation?

Can't dummy32+1 be 4097?

--b.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/