netdev - RE: [RFC V2 PATCH] rtnetlink: Fix problem with buffer allocation

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]

Message-ID: <C5551D9AAB213A418B7FD5E4A6F30A0702F8C76C@ORSMSX106.amr.corp.intel.com>
Date:	Tue, 14 Feb 2012 21:34:48 +0000
From:	"Rose, Gregory V" <gregory.v.rose@...el.com>
To:	David Miller <davem@...emloft.net>
CC:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [RFC V2 PATCH] rtnetlink: Fix problem with buffer allocation

> -----Original Message-----
> From: David Miller [mailto:davem@...emloft.net]
> Sent: Tuesday, February 14, 2012 1:23 PM
> To: Rose, Gregory V
> Cc: netdev@...r.kernel.org
> Subject: Re: [RFC V2 PATCH] rtnetlink: Fix problem with buffer allocation
> 
> From: Greg Rose <gregory.v.rose@...el.com>
> Date: Sun, 12 Feb 2012 11:13:42 -0800
> 
> > I have kept the NLM_F_EXT nlmsg_flags bit to indicate to the kernel
> > that the extended ifinfo dump filter mask is present.
> 
> No extra indications other than presence of the attribute itself is
> necessary.  Please remove this flag.
> 
> > @@ -215,6 +216,7 @@ int netlink_sendskb(struct sock *sk, struct sk_buff
> *skb);
> >  #else
> >  #define NLMSG_GOODSIZE	SKB_WITH_OVERHEAD(8192UL)
> >  #endif
> > +#define NLMSG_EXT_GOODSIZE SKB_WITH_OVERHEAD(32768UL)
> 
> I indicated in my suggestions that you'll need to calculate this at
> run-time based upon the extensions enabled and the size of the resulting
> message plus attributes.
> 
> You absolutely cannot just pick some large number and run with this,
> it's highly wasteful and will be potentially causing allocation
> failures.
> 
> The goodsize value is specifically choosen such that we don't exceed
> an order-1 allocation with page sizes of 4096 and larger.

If I do the buffer size calculation based upon the extensions enabled then I'll have to assume the maximum possible size because at the time the buffer size is calculated we don't' have a handle to the net devices yet.

So if the VF flag is set then I'll assume that up to 255 VFs might be present and calculate based upon that.

> 
> > +struct rtnl_req_extended {
> > +	struct nlmsghdr nlh;
> > +	struct rtgenmsg g;
> > +	char ext[RTA_SPACE(sizeof(__u32))];
> > +};
> > +
> > +/* New extended info filters for IFLA_EXT_MASK */
> > +#define RTEXT_FILTER_VF		(1 << 0)
> > +
> 
> This seems completely unnecessary.
> 
> Just define IFLA_EXT_MASK as a variable length array of u32's, but to
> be honest, for now, you can just make it a normal u32 attribute.
> 
> If we extend it in the future, we can make the kernel handle any
> length, u32 or otherwise.  The size field already present in all
> netlink attributes will allow us to do this transparently.

OK, I think I see where you and Ben are coming from on this.

Thanks,

- Greg

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html