netdev - Re: [RFC][PATCH net-next] ethtool: Allow drivers to select RX NFC rule locations

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] [day] [month] [year] [list]

Date:	Sat, 17 Dec 2011 04:15:01 +0000
From:	Ben Hutchings <bhutchings@...arflare.com>
To:	Dimitris Michailidis <dm@...lsio.com>
CC:	netdev <netdev@...r.kernel.org>,
	<linux-net-drivers@...arflare.com>,
	Alexander Duyck <alexander.h.duyck@...el.com>,
	Vladislav Zolotarov <vladz@...adcom.com>
Subject: Re: [RFC][PATCH net-next] ethtool: Allow drivers to select RX NFC
 rule locations

On Fri, 2011-12-16 at 18:44 -0800, Dimitris Michailidis wrote:
> On 12/16/2011 05:18 PM, Ben Hutchings wrote:
> > Define special location values for RX NFC that request the driver to
> > select the actual rule location.  This allows for implementation on
> > devices that use hash-based filter lookup, whereas currently the API is
> > more suited to devices with TCAM lookup or linear search.
> > 
> > In ethtool_set_rxnfc() and the compat wrapper ethtool_ioctl(), copy
> > the structure back to user-space after insertion so that the actual
> > location is returned.
> > 
> > Signed-off-by: Ben Hutchings <bhutchings@...arflare.com>
> 
> I like this change.  One concern below.
> 
> > -	return dev->ethtool_ops->set_rxnfc(dev, &info);
> > +	rc = dev->ethtool_ops->set_rxnfc(dev, &info);
> > +	if (rc)
> > +		return rc;
> > +
> > +	if (cmd == ETHTOOL_SRXCLSRLINS &&
> > +	    copy_to_user(useraddr, &info, info_size))
> > +		return -EFAULT;
> 
> Here we return failure but the rule has been added successfully and is in 
> effect.  It may be better to return 0 and let user-space tell this last step 
> failed by the fact that the location field is still special.

If copy_to_user() fails then the user program has a bug (or is probing
for security flaws).  A return value of -EFAULT is morally equivalent to
SIGSEGV.  (I'm not sure why it isn't translated into a signal on return,
but I imagine there are historical reasons.)  I don't see any point in
trying to help userland recover from this.

Ben.

-- 
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

--
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