Netdev List
 help / color / mirror / Atom feed
* [PATCH v1] net/core: support runtime PM on net_device
From: Ming Lei @ 2012-10-18  8:21 UTC (permalink / raw)
  To: David S. Miller, Rafael J. Wysocki
  Cc: Oliver Neukum, Alan Stern, netdev, linux-pm, Ming Lei

In ioctl path on net_device, the physical deivce is often
touched, but the physical device may have been put into runtime
suspend state already, so cause some utilitis(ifconfig, ethtool,
...) to return failure in this situation.

This patch enables runtime PM on net_device and mark it as
no_callbacks, and resumes the net_device if physical device
is to be accessed, then suspends it after completion of the
access.

This patch fixes the problem above.

Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
v1:
	- fix one undefined local variable compile failure

 net/core/dev.c       |   83 +++++++++++++++++++++++++++++++-------------------
 net/core/ethtool.c   |    9 ++++--
 net/core/net-sysfs.c |    4 +++
 3 files changed, 63 insertions(+), 33 deletions(-)

diff --git a/net/core/dev.c b/net/core/dev.c
index 09cb3f6..d772ce8 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -135,6 +135,7 @@
 #include <linux/net_tstamp.h>
 #include <linux/static_key.h>
 #include <net/flow_keys.h>
+#include <linux/pm_runtime.h>
 
 #include "net-sysfs.h"
 
@@ -4993,39 +4994,24 @@ static int dev_ifsioc_locked(struct net *net, struct ifreq *ifr, unsigned int cm
 /*
  *	Perform the SIOCxIFxxx calls, inside rtnl_lock()
  */
-static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
+static int __dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
 {
 	int err;
 	struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
 	const struct net_device_ops *ops;
 
-	if (!dev)
-		return -ENODEV;
-
 	ops = dev->netdev_ops;
 
 	switch (cmd) {
 	case SIOCSIFFLAGS:	/* Set interface flags */
 		return dev_change_flags(dev, ifr->ifr_flags);
 
-	case SIOCSIFMETRIC:	/* Set the metric on the interface
-				   (currently unused) */
-		return -EOPNOTSUPP;
-
 	case SIOCSIFMTU:	/* Set the MTU of a device */
 		return dev_set_mtu(dev, ifr->ifr_mtu);
 
 	case SIOCSIFHWADDR:
 		return dev_set_mac_address(dev, &ifr->ifr_hwaddr);
 
-	case SIOCSIFHWBROADCAST:
-		if (ifr->ifr_hwaddr.sa_family != dev->type)
-			return -EINVAL;
-		memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
-		       min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
-		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
-		return 0;
-
 	case SIOCSIFMAP:
 		if (ops->ndo_set_config) {
 			if (!netif_device_present(dev))
@@ -5049,21 +5035,6 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
 		if (!netif_device_present(dev))
 			return -ENODEV;
 		return dev_mc_del_global(dev, ifr->ifr_hwaddr.sa_data);
-
-	case SIOCSIFTXQLEN:
-		if (ifr->ifr_qlen < 0)
-			return -EINVAL;
-		dev->tx_queue_len = ifr->ifr_qlen;
-		return 0;
-
-	case SIOCSIFNAME:
-		ifr->ifr_newname[IFNAMSIZ-1] = '\0';
-		return dev_change_name(dev, ifr->ifr_newname);
-
-	case SIOCSHWTSTAMP:
-		err = net_hwtstamp_validate(ifr);
-		if (err)
-			return err;
 		/* fall through */
 
 	/*
@@ -5099,6 +5070,56 @@ static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
 	return err;
 }
 
+static int dev_ifsioc(struct net *net, struct ifreq *ifr, unsigned int cmd)
+{
+	int err;
+	struct net_device *dev = __dev_get_by_name(net, ifr->ifr_name);
+	const struct net_device_ops *ops;
+
+	if (!dev)
+		return -ENODEV;
+
+	ops = dev->netdev_ops;
+
+	switch (cmd) {
+	case SIOCSIFMETRIC:	/* Set the metric on the interface
+				   (currently unused) */
+		return -EOPNOTSUPP;
+
+	case SIOCSIFHWBROADCAST:
+		if (ifr->ifr_hwaddr.sa_family != dev->type)
+			return -EINVAL;
+		memcpy(dev->broadcast, ifr->ifr_hwaddr.sa_data,
+		       min(sizeof ifr->ifr_hwaddr.sa_data, (size_t) dev->addr_len));
+		call_netdevice_notifiers(NETDEV_CHANGEADDR, dev);
+		return 0;
+
+	case SIOCSIFTXQLEN:
+		if (ifr->ifr_qlen < 0)
+			return -EINVAL;
+		dev->tx_queue_len = ifr->ifr_qlen;
+		return 0;
+
+	case SIOCSIFNAME:
+		ifr->ifr_newname[IFNAMSIZ-1] = '\0';
+		return dev_change_name(dev, ifr->ifr_newname);
+
+	case SIOCSHWTSTAMP:
+		err = net_hwtstamp_validate(ifr);
+		if (err)
+			return err;
+	}
+
+	if (pm_runtime_get_sync(&dev->dev) < 0)
+		return -ENODEV;
+
+	err = __dev_ifsioc(net, ifr, cmd);
+
+	pm_runtime_put(&dev->dev);
+
+	return err;
+}
+
 /*
  *	This function handles all "interface"-type I/O control requests. The actual
  *	'doing' part of this is dev_ifsioc above.
diff --git a/net/core/ethtool.c b/net/core/ethtool.c
index 4d64cc2..dae4417 100644
--- a/net/core/ethtool.c
+++ b/net/core/ethtool.c
@@ -25,6 +25,7 @@
 #include <linux/slab.h>
 #include <linux/rtnetlink.h>
 #include <linux/sched.h>
+#include <linux/pm_runtime.h>
 
 /*
  * Some useful ethtool_ops methods that're device independent.
@@ -1464,10 +1465,13 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 			return -EPERM;
 	}
 
+	if (pm_runtime_get_sync(&dev->dev) < 0)
+		return -ENODEV;
+
 	if (dev->ethtool_ops->begin) {
 		rc = dev->ethtool_ops->begin(dev);
 		if (rc  < 0)
-			return rc;
+			goto exit;
 	}
 	old_features = dev->features;
 
@@ -1648,6 +1652,7 @@ int dev_ethtool(struct net *net, struct ifreq *ifr)
 
 	if (old_features != dev->features)
 		netdev_features_change(dev);
-
+exit:
+	pm_runtime_put(&dev->dev);
 	return rc;
 }
diff --git a/net/core/net-sysfs.c b/net/core/net-sysfs.c
index bcf02f6..c9adb89 100644
--- a/net/core/net-sysfs.c
+++ b/net/core/net-sysfs.c
@@ -23,6 +23,7 @@
 #include <linux/export.h>
 #include <linux/jiffies.h>
 #include <net/wext.h>
+#include <linux/pm_runtime.h>
 
 #include "net-sysfs.h"
 
@@ -1415,6 +1416,9 @@ int netdev_register_kobject(struct net_device *net)
 	if (error)
 		return error;
 
+	pm_runtime_no_callbacks(dev);
+	pm_runtime_enable(dev);
+
 	error = register_queue_kobjects(net);
 	if (error) {
 		device_del(dev);
-- 
1.7.9.5

^ permalink raw reply related

* Re: [PATCH v1] net/core: support runtime PM on net_device
From: Bjørn Mork @ 2012-10-18  8:29 UTC (permalink / raw)
  To: Ming Lei
  Cc: David S. Miller, Rafael J. Wysocki, Oliver Neukum, Alan Stern,
	netdev, linux-pm
In-Reply-To: <1350548469-1267-1-git-send-email-ming.lei@canonical.com>

Ming Lei <ming.lei@canonical.com> writes:

> In ioctl path on net_device, the physical deivce is often
> touched, but the physical device may have been put into runtime
> suspend state already, so cause some utilitis(ifconfig, ethtool,
> ...) to return failure in this situation.

I have to as the stupid questions again, sorry...

Just wondering, isn't that really a driver problem?  The driver will
know whether or not hardware access is required, and should wake up the
device if necessary.  Unless I misunderstand something here, this seems
like papering over driver bugs?


Bjørn

^ permalink raw reply

* RE: [PATCH] SUNRPC: Prevent kernel stack corruption on long values of flush
From: David Laight @ 2012-10-18  8:34 UTC (permalink / raw)
  To: Boaz Harrosh, Jim Rees
  Cc: Dave Jones, J. Bruce Fields, Sasha Levin,
	Trond.Myklebust-HgOvQuBEEgTQT0dZR+AlfA,
	davem-fT/PcQaiUtIeIZ0/mPfg9Q, linux-nfs-u79uwXL29TY76Z2rM5mHXA,
	netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <507EF918.5030004-C4P08NqkoRlBDgjK7y7TUQ@public.gmane.org>

> ...
> long is always the same or bigger then a pointer
> (A pointer must always fit in a long)
> ...

Linux may make that assumption, but it doesn't have
to be true. 64bit windows still has 32bit long.
C99 inttypes.h defines [u]intptr_t to be an integral type
that is large enough to hold a pointer to any data item.
(That in itself is problematic for implementations that
encode multiple characters into a machine word and need
to use 'fat' pointers in order to encode the offset.)

	David


^ permalink raw reply

* RE: Bug?  TCP shutdown behaviour when deleting local IP addresses
From: David Laight @ 2012-10-18  9:00 UTC (permalink / raw)
  To: Mikael Abrahamsson, Chris Friesen
  Cc: netdev, David Miller, Alexey Kuznetsov, James Morris,
	Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <alpine.DEB.2.00.1210180946020.21297@uplift.swm.pp.se>

> 2. When a network connection (physical interface) goes down, wait a few
> seconds, give up, reset all connectivity related to that connection,
> basically give up.

Doing that is almost pointless and gives people false expectations.

Consider the simple network host-hub-hub-host.
If you disconnect the link between the two hubs then
only timeouts can cause disconnects.
Such a remote fault is much more likely than the local one.

If the kernel takes down connections when a local interface
goes down (as windows does) then people doing testing
fail to test the correct scenario.

You also don't want a power glitch in the wiring cupboard
to disconnect all your connections.

In the past I've also moved IP addresses between physical
interfaces - modern HA systems might do the same.

	David

^ permalink raw reply

* sock_getsockopt() not exported
From: David Laight @ 2012-10-18  9:11 UTC (permalink / raw)
  To: netdev

I've noticed that net/core/sock.c contains an
    EXPORT_SYMBOL(sock_setsockopt)
but is missing the corresponding
    EXPORT_SYMBOL(sock_getsockopt)

In-kernel users of sockets probably manage without
needing to read SOL_SOCKET options.
(They do need to set SO_REUSADDR and SO_KEEPALIVE.)

	David

^ permalink raw reply

* Re: Bug?  TCP shutdown behaviour when deleting local IP addresses
From: Eric Dumazet @ 2012-10-18  9:13 UTC (permalink / raw)
  To: Mikael Abrahamsson
  Cc: Chris Friesen, netdev, David Miller, Alexey Kuznetsov,
	James Morris, Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <alpine.DEB.2.00.1210180946020.21297@uplift.swm.pp.se>

On Thu, 2012-10-18 at 10:05 +0200, Mikael Abrahamsson wrote:
> On Wed, 17 Oct 2012, Chris Friesen wrote:
> 
> > 1) create new IP address and assign to eth device
> > 2) TCP server starts listening on that IP address
> > 3) TCP client connects to server
> > 4) remove new IP address
> 
> I'm a network engineer, as in I work primarily with IP routing. Ever since 
> I started running Linux back in the mid 90ties I've had a love/hate 
> relationship with how Linux handles disappearing network connectivity or 
> IP addresses.
> 
> In my mind there are two ways to handle outage:
> 
> 1. When a network connection (physical interface) goes down, keep 
> everything as it is, it might come back up again shortly and then we can 
> continue as if basically nothing happened. TCP was designed for this 
> considering timeouts can be in hours.
> 
> 2. When a network connection (physical interface) goes down, wait a few 
> seconds, give up, reset all connectivity related to that connection, 
> basically give up.
> 
> Now to my question for the netdev people:
> 
> Is there functionality in the kernel for a connection manager to easily 
> accomplish 2, in that when it tries to deconfigure the IP address on the 
> interface to also kill all TCP connections terminated at that IP? On my 
> laptop, I regularily have to kill my ssh client after suspend/resume 
> cycle, because it's been down for quite a while, and the ssh client 
> doesn't know the TCP connection is now not functional anymore (TCP session 
> is still up and retransmit won't happen for a while, so the TCP RST from 
> the server (I use keepalives within SSH) isn't seen for a long time).
> 
> Without knowing what's in place right now, I see some behaviours that I'd 
> like to have:
> 
> After resume (or otherwise network connectivity re-established), 
> connection manager should be able to tell the kernel to:
> 
> a) kill all TCP/UDP/other sessions existing which doesn't currently have 
> an active IP address on the machine. This is for the sake of local 
> clients.

You do realize kernel has no idea that the loss of IP address is
temporary or not ?

Some links can be slow to setup after a resume.

> b) the TCP/SCTP sessions that *do* have an IP, should have their 
> retransmit timers "reset", so that whatever needs to be sent, should be 
> sent immediately (or shortly, within a few seconds).

So they are going to force a close, even if the link becomes alive after
15 seconds. Too bad for some wireless setups, or tunnels.

> c) tell the kernel to kill all TCP sessions bound to a certain IP, because 
> the connection manager is going to remove it shortly. Send TCP RSTs or 
> whatever and close the TCP session, so both ends know that network 
> connectivity is going down.
> 

Yes, why not. Why is it specific to linux I have no idea.

^ permalink raw reply

* [ANNOUNCE] iptables 1.4.16.3 release
From: Pablo Neira Ayuso @ 2012-10-18  9:26 UTC (permalink / raw)
  To: netfilter-devel; +Cc: netdev, netfilter, lwn

[-- Attachment #1: Type: text/plain, Size: 323 bytes --]

Hi!

The Netfilter project proudly presents:

        iptables 1.4.16.3

This release fixes the static builds and compile on RHEL5.

See ChangeLog that comes attached to this email for more details.

You can download it from:

http://www.netfilter.org/projects/iptables/downloads.html
ftp://ftp.netfilter.org/pub/iptables/

[-- Attachment #2: changes-iptables-1.4.16.3.txt --]
[-- Type: text/plain, Size: 204 bytes --]

Jan Engelhardt (2):
      build: remove symlink-only extensions from static object list
      build: resolve compile abort in libxt_limit on RHEL5

Pablo Neira Ayuso (1):
      bump iptables to 1.4.16.3


^ permalink raw reply

* Re: sock_getsockopt() not exported
From: Eric Dumazet @ 2012-10-18  9:30 UTC (permalink / raw)
  To: David Laight; +Cc: netdev
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7052@saturn3.aculab.com>

On Thu, 2012-10-18 at 10:11 +0100, David Laight wrote:
> I've noticed that net/core/sock.c contains an
>     EXPORT_SYMBOL(sock_setsockopt)
> but is missing the corresponding
>     EXPORT_SYMBOL(sock_getsockopt)
> 
> In-kernel users of sockets probably manage without
> needing to read SOL_SOCKET options.
> (They do need to set SO_REUSADDR and SO_KEEPALIVE.)


sock_setsockopt() is exported because sunrpc needs it, and sunrpc can be
a module

sock_getsockopt() is not exported because no module needs it yet.

The day one user needs it, we'll add the EXPORT_SYMBOL()

^ permalink raw reply

* Re: Bug?  TCP shutdown behaviour when deleting local IP addresses
From: Mikael Abrahamsson @ 2012-10-18  9:58 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Chris Friesen, netdev, David Miller, Alexey Kuznetsov,
	James Morris, Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <1350551609.26103.1261.camel@edumazet-glaptop>

On Thu, 18 Oct 2012, Eric Dumazet wrote:

>> After resume (or otherwise network connectivity re-established),
>> connection manager should be able to tell the kernel to:
>>
>> a) kill all TCP/UDP/other sessions existing which doesn't currently have
>> an active IP address on the machine. This is for the sake of local
>> clients.
>
> You do realize kernel has no idea that the loss of IP address is
> temporary or not ?

Indeed, that's why I wrote "connection manager tell the kernel to..", and 
didn't write "the kernel should do".

> Some links can be slow to setup after a resume.

Absolutely.

>> b) the TCP/SCTP sessions that *do* have an IP, should have their
>> retransmit timers "reset", so that whatever needs to be sent, should be
>> sent immediately (or shortly, within a few seconds).
>
> So they are going to force a close, even if the link becomes alive after
> 15 seconds. Too bad for some wireless setups, or tunnels.

I don't get what you're saying. I'm saying reset the retransmit timers, if 
there is no response, exponential backoff applies again. The reset of the 
timers is to avoid that the exponential backoff timer is now in "wait for 
several minutes to send the next packet" due to no connectivity during the 
first 10-20 seconds after coming out of resume.

On my Ubuntu 12.04 system, when I bring it out of resume and press enter 
in the terminal with an established ssh session, it takes minutes to 
discover that this session doesn't work and close it (it seems the TCP RST 
from the server isn't triggered for a long time). I usually end up killing 
the ssh process on the laptop and logging in again. What I want to happen 
is that within a few seconds of network connectivity being established, 
TCP should try to communicate and discover that the other end already 
closed the connection and close the local socket call so ssh quits and I 
can re-login again. One way of doing this would be for the connection 
manager to ask the kernel to reset (or severely lower) the retransmit 
timers on all established TCP connections.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* RE: Bug?  TCP shutdown behaviour when deleting local IP addresses
From: Mikael Abrahamsson @ 2012-10-18 10:00 UTC (permalink / raw)
  To: David Laight
  Cc: Chris Friesen, netdev, David Miller, Alexey Kuznetsov,
	James Morris, Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <AE90C24D6B3A694183C094C60CF0A2F6026B7051@saturn3.aculab.com>

On Thu, 18 Oct 2012, David Laight wrote:

> Doing that is almost pointless and gives people false expectations.
>
> Consider the simple network host-hub-hub-host.
> If you disconnect the link between the two hubs then
> only timeouts can cause disconnects.
> Such a remote fault is much more likely than the local one.

My doing suspect/resume on my laptop causes more network interruptions 
than any link going down in the network outside my home.

> If the kernel takes down connections when a local interface
> goes down (as windows does) then people doing testing
> fail to test the correct scenario.

I never asked for the kernel to take down connections, I asked for the 
connection manager to be able to do it.

> You also don't want a power glitch in the wiring cupboard to disconnect 
> all your connections.

In a server environment, most likely you're right. In some other 
environment, the opposite probably applies.

> In the past I've also moved IP addresses between physical
> interfaces - modern HA systems might do the same.

Yes, in your described scenario you're right, in my scenario (laptop 
suspend/resume) I don't agree with your view.

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Re: Bug?  TCP shutdown behaviour when deleting local IP addresses
From: Alexey Kuznetsov @ 2012-10-18 10:01 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: Mikael Abrahamsson, Chris Friesen, netdev, David Miller,
	James Morris, Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <1350551609.26103.1261.camel@edumazet-glaptop>

On Thu, Oct 18, 2012 at 11:13:29AM +0200, Eric Dumazet wrote:
> > c) tell the kernel to kill all TCP sessions bound to a certain IP, because 
> > the connection manager is going to remove it shortly. Send TCP RSTs or 
> > whatever and close the TCP session, so both ends know that network 
> > connectivity is going down.
> > 
> 
> Yes, why not.

FYI the idea was by Andi Kleen back in 2003. If was flag IFF_DYNAMIC
on device (apparently, it should be per-interface sysctl instead
or even a flag on specific address).

Andi suggested to hook netdev notifier and to reset tcp connections
bound to addresses on this interface. He did not go so far to send
resets before address is actually disabled (it was not a goal, normally
address is already dead to the time when it is deleted),
but techically it is the same.

The problem with this was purely technical, the code has to scan through
all the tcp hash table to search for connections to this address (grrr already :-))
and to take socket lock before making any actions. It is doable, but quite chumbersome
and nobody was interested enough to finish the job.

^ permalink raw reply

* [PATCH 0/5] drivers: xen frontend devices should handle missed backend CLOSING
From: David Vrabel @ 2012-10-18 10:02 UTC (permalink / raw)
  To: xen-devel@lists.xensource.com, linux-kernel@vger.kernel.org,
	netdev@vger.kernel.org, David S. Miller, Jens Axboe, linux-pci,
	Bjorn Helgaas, linux-fbdev, Florian Tobias Schandinat,
	linux-input, Dmitry Torokhov
  Cc: David Vrabel

Subsystem maintainers, you can either pick up the relevant driver patch
or ack it to go via Konrad's Xen tree.

The series makes all the Xen frontend drivers handle the backend
transitioning to CLOSED without the frontend having previously seen
the backend in the CLOSING state.

Backends shouldn't do this but some do.  e.g., if the host is
XenServer and the toolstack decides to do a forced shutdown of a VBD,
then the blkfront may miss the CLOSING transition and the /dev/xvdX
device will not be destroyed which prevents it being reused.

I have seen systems that ended up in this state but it's not clear if
this was the actual cause.  However, I think in general it's a good
thing to thing to improve the handling of unexpected state
transitions.

David

^ permalink raw reply

* [PATCH 1/5] xen-netfront: handle backend CLOSED without CLOSING
From: David Vrabel @ 2012-10-18 10:03 UTC (permalink / raw)
  To: xen-devel
  Cc: David Vrabel, Konrad Rzeszutek Wilk, linux-kernel, netdev,
	David S. Miller
In-Reply-To: <507FD39F.4060601@citrix.com>

From: David Vrabel <david.vrabel@citrix.com>

Backend drivers shouldn't transistion to CLOSED unless the frontend is
CLOSED.  If a backend does transition to CLOSED too soon then the
frontend may not see the CLOSING state and will not properly shutdown.

So, treat an unexpected backend CLOSED state the same as CLOSING.

Signed-off-by: David Vrabel <david.vrabel@citrix.com>
Acked-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: netdev@vger.kernel.org
Cc: "David S. Miller" <davem@davemloft.net>
---
 drivers/net/xen-netfront.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/drivers/net/xen-netfront.c b/drivers/net/xen-netfront.c
index caa0110..40cde51 100644
--- a/drivers/net/xen-netfront.c
+++ b/drivers/net/xen-netfront.c
@@ -1701,7 +1701,6 @@ static void netback_changed(struct xenbus_device *dev,
 	case XenbusStateReconfiguring:
 	case XenbusStateReconfigured:
 	case XenbusStateUnknown:
-	case XenbusStateClosed:
 		break;
 
 	case XenbusStateInitWait:
@@ -1716,6 +1715,10 @@ static void netback_changed(struct xenbus_device *dev,
 		netdev_notify_peers(netdev);
 		break;
 
+	case XenbusStateClosed:
+		if (dev->state == XenbusStateClosed)
+			break;
+		/* Missed the backend's CLOSING state -- fallthrough */
 	case XenbusStateClosing:
 		xenbus_frontend_closed(dev);
 		break;
-- 
1.7.2.5

^ permalink raw reply related

* Re: Bug?  TCP shutdown behaviour when deleting local IP addresses
From: Mikael Abrahamsson @ 2012-10-18 10:10 UTC (permalink / raw)
  To: Alexey Kuznetsov
  Cc: Eric Dumazet, Chris Friesen, netdev, David Miller, James Morris,
	Patrick McHardy, Hideaki YOSHIFUJI
In-Reply-To: <20121018100149.GA5176@ms2.inr.ac.ru>

On Thu, 18 Oct 2012, Alexey Kuznetsov wrote:

> Andi suggested to hook netdev notifier and to reset tcp connections 
> bound to addresses on this interface. He did not go so far to send 
> resets before address is actually disabled (it was not a goal, normally 
> address is already dead to the time when it is deleted), but techically 
> it is the same.

With IPv6 and temporary addresses etc, and DHCPv4/v6 leases expiring, 
addresses going away can be a quite foreseeable event. When I suspend my 
laptop, the IP going away is also quite forseeable.

> The problem with this was purely technical, the code has to scan through 
> all the tcp hash table to search for connections to this address (grrr 
> already :-)) and to take socket lock before making any actions. It is 
> doable, but quite chumbersome and nobody was interested enough to finish 
> the job.

Are there hooks so that someone could write a userland application to do 
this job? I could imagine the connection manager doing this job as well 
(so it's configurable what connection to close or not).

What about the reset of the TCP retransmit timers, is that doable from 
userland? Or at least lowering them to 1s or something fairly sane (going 
to fast-retransmit doesn't make sense, neither does having a retransmit 
timer in the 10s of minutes).

Btw, what are the TCP retransmit timers when I come out of suspend? The 
machine has been "offline" for hours, what idea does TCP have of what's 
going on, will it keep the timers it had when machine went down or does 
TCP believe it hasn't seen a response for a long time and now the 
retransmit timers are very high?

-- 
Mikael Abrahamsson    email: swmike@swm.pp.se

^ permalink raw reply

* Fwd: Re: [PATCH v3] net-tcp: TCP/IP stack bypass for loopback connections
From: Weiping Pan @ 2012-10-18 10:19 UTC (permalink / raw)
  To: open list:NETWORKING [GENERAL]

Sorry, forget to cc the list.

2012/9/18 Bruce "Brutus" Curtis<brutus@google.com>:
>  From: "Bruce \"Brutus\" Curtis"<brutus@google.com>
>
>  TCP/IP loopback socket pair stack bypass, based on an idea by, and
>  rough upstream patch from, David Miller<davem@davemloft.net>  called
>  "friends", the data structure modifcations and connection scheme are
>  reused with extensive data-path changes.

Hi, Bruce,

I found that there is a bug in the tcp friends patch,
when I kill netperf randomly, panic occurs in tcp_close().

BUG: unable to handle kernel NULL pointer dereference at 0000000d
IP: [<c0835a9b>] tcp_close+0x7b/0x3a0
*pde = 00000000
Oops: 0000 [#1] SMP
Modules linked in: fuse 8021q garp stp llc ip6t_REJECT
nf_conntrack_ipv6 nf_defrag_ipv6 nf_conntrack_ipv4 nf_defrag_ipv4
ip6table_filter xt_state nf_conntrack ip6_tables ppdev parport_pc
pcspkr i2c_piix4 i2c_core parport microcode e1000 uinput
Pid: 16627, comm: netperf Not tainted 3.6.0+ #25 innotek GmbH VirtualBox
EIP: 0060:[<c0835a9b>] EFLAGS: 00010202 CPU: 1
EIP is at tcp_close+0x7b/0x3a0
EAX: f6f41240 EBX: c2a46f40 ECX: 00000000 EDX: 00000001
ESI: 00000000 EDI: c2a46f88 EBP: c2a1fd8c ESP: c2a1fd78
  DS: 007b ES: 007b FS: 00d8 GS: 00e0 SS: 0068
CR0: 8005003b CR2: 0000000d CR3: 00c07000 CR4: 000006d0
DR0: 00000000 DR1: 00000000 DR2: 00000000 DR3: 00000000
DR6: ffff0ff0 DR7: 00000400
Process netperf (pid: 16627, ti=c2a1e000 task=f44cd780 task.ti=c2a1e000)
Stack:
  c0b4d880 00000000 c2a46f40 ed821080 f4de7f00 c2a1fd9c c0857cff ed821080
  00000000 c2a1fdb0 c07e3db0 00000000 f6f07540 00000008 c2a1fdbc c07e4127
  f4de7f00 c2a1fdec c05378e8 00000001 00000000 00000000 f54bf010 ed82109c
Call Trace:
  [<c0857cff>] inet_release+0x5f/0x70
  [<c07e3db0>] sock_release+0x20/0x80
  [<c07e4127>] sock_close+0x17/0x30
  [<c05378e8>] __fput+0x98/0x1f0
  [<c0537a4d>] ____fput+0xd/0x10
  [<c04580f1>] task_work_run+0x91/0xb0
  [<c0441157>] do_exit+0x177/0x7f0
  [<c0422c97>] ? smp_reschedule_interrupt+0x27/0x30
  [<c0441a67>] do_group_exit+0x37/0xa0
  [<c044e989>] get_signal_to_deliver+0x1c9/0x5b0
  [<c0471393>] ? update_curr+0x213/0x380
  [<c0402bca>] do_signal+0x2a/0x980
  [<c04026c7>] ? __switch_to+0xc7/0x340
  [<c08ea8c9>] ? __schedule+0x379/0x780
  [<c08ec3f8>] ? apic_timer_interrupt+0x34/0x3c
  [<c04ad0ae>] ? __audit_syscall_exit+0x36e/0x3a0
  [<c04ad0ae>] ? __audit_syscall_exit+0x36e/0x3a0
  [<c04036e5>] do_notify_resume+0x75/0xa0
  [<c08ec1c1>] work_notifysig+0x30/0x37
Code: 85 c0 74 3e 83 6b 50 01 8b 08 8b 50 04 c7 00 00 00 00 00 c7 40
04 00 00 00 00 89 51 04 89 0a 8b 88 9c 00 00 00 8b 50 34 2b 50 30<0f>
b6 49 0d 83 e1 01 29 ca 01 d6 e8 c5 64 fb ff 8b 43 48 39 f8
EIP: [<c0835a9b>] tcp_close+0x7b/0x3a0 SS:ESP 0068:c2a1fd78
CR2: 000000000000000d
---[ end trace 9f6d5c8fc973265c ]---


How to reproduce it ?
1 run netserver in a loop
2 run netperf with different modes in a loop
3 kill netperf randomly

And I found it is easy to see the panic on VirtualBox, but I did not
see it on the real machine.

Any hints ?

thanks
Weiping Pan

^ permalink raw reply

* [PATCH v7 00/10] IPC: checkpoint/restore in userspace enhancements
From: Stanislav Kinsbursky @ 2012-10-18 10:22 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey

v7:
1) Added comments in places, when behiviour is not obvious.
2) Compilation fixed for compat layer
3) test updated to use existent header files (instead of hard-coding new
defines in case of absence).
4) comment fixed in qlge driver

v6:
1) rebased on 3.7-rc1

v5:
1) Several define-dependent compile bugs fixed
2) IPC message copy test updated
3) A couple of minor fixes.
4) Qlogic driver update: rename of its internal SEM_SET define into SEM_INIT
(compile error).

v4:
1) If MSG_COPY flag is specified, then "mtype" is not a type, but message
number to copy.
2) MSG_SET_COPY logic for sys_msgctl() was removed.

v3:
1) Copy messages to user-space under spinlock was replaced by allocation of
dummy message before queue lock and then copy of desired message to the dummy
one instead of unlinking it from queue list.
I.e. the message queue copy logic was changed: messages can be retrived one by
one (instead of receiving of the whole list at once).

This patch set is aimed to provide additional functionality for all IPC
objects,
which is required for migration of these objects by user-space
checkpoint/restore utils (CRIU).

The main problem here was impossibility to set up object id. This patch set
solves the problem in two steps:
1) Makes it possible to create new object (shared memory, semaphores set or
messages queue) with ID, equal to passed key.
2) Makes it possible to change existent object key.

Another problem was to peek messages from queues without deleting them.
This was achived by introducing of new MSG_COPY flag for sys_msgrcv(). If
MSG_COPY flag is set, then msgtyp is interpreted as message number.

The following series implements...

---

Stanislav Kinsbursky (10):
      ipc: remove forced assignment of selected message
      ipc: "use key as id" functionality for resource get system call introduced
      ipc: segment key change helper introduced
      ipc: add new SHM_SET command for sys_shmctl() call
      ipc: add new MSG_SET command for sys_msgctl() call
      qlge driver: rename internal SEM_SET macro to SEM_INIT
      ipc: add new SEM_SET command for sys_semctl() call
      IPC: message queue receive cleanup
      IPC: message queue copy feature introduced
      test: IPC message queue copy feture test


 drivers/net/ethernet/qlogic/qlge/qlge.h      |    4 
 drivers/net/ethernet/qlogic/qlge/qlge_main.c |   16 +-
 include/linux/msg.h                          |    5 -
 include/uapi/linux/ipc.h                     |    1 
 include/uapi/linux/msg.h                     |    2 
 include/uapi/linux/sem.h                     |    1 
 include/uapi/linux/shm.h                     |    1 
 ipc/compat.c                                 |   54 +++---
 ipc/msg.c                                    |  117 ++++++++++---
 ipc/msgutil.c                                |   38 ++++
 ipc/sem.c                                    |   15 +-
 ipc/shm.c                                    |   18 ++
 ipc/util.c                                   |   67 +++++++-
 ipc/util.h                                   |    6 +
 security/selinux/hooks.c                     |    3 
 security/smack/smack_lsm.c                   |    3 
 tools/testing/selftests/ipc/Makefile         |   25 +++
 tools/testing/selftests/ipc/msgque.c         |  231 ++++++++++++++++++++++++++
 18 files changed, 526 insertions(+), 81 deletions(-)
 create mode 100644 tools/testing/selftests/ipc/Makefile
 create mode 100644 tools/testing/selftests/ipc/msgque.c

^ permalink raw reply

* [PATCH v7 01/10] ipc: remove forced assignment of selected message
From: Stanislav Kinsbursky @ 2012-10-18 10:22 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

This is a cleanup patch. The assignment is redundant.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 ipc/msg.c |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index a71af5a..2f272fa 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -793,12 +793,9 @@ long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
 				msg = walk_msg;
 				if (mode == SEARCH_LESSEQUAL &&
 						walk_msg->m_type != 1) {
-					msg = walk_msg;
 					msgtyp = walk_msg->m_type - 1;
-				} else {
-					msg = walk_msg;
+				} else
 					break;
-				}
 			}
 			tmp = tmp->next;
 		}

^ permalink raw reply related

* [PATCH v7 02/10] ipc: "use key as id" functionality for resource get system call introduced
From: Stanislav Kinsbursky @ 2012-10-18 10:22 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

This patch introduces new IPC resource get request flag IPC_PRESET, which
should be interpreted as a request to try to allocate IPC slot with number,
starting from value resented by key. IOW, kernel will try
allocate new segment in specified slot.

Note: if desired slot is not emply, then next free slot will be used.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 include/uapi/linux/ipc.h |    1 +
 ipc/msg.c                |    4 +++-
 ipc/sem.c                |    4 +++-
 ipc/shm.c                |    4 +++-
 ipc/util.c               |   18 +++++++++++++++---
 ipc/util.h               |    3 ++-
 6 files changed, 27 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/ipc.h b/include/uapi/linux/ipc.h
index de08dd4..f5f52b6 100644
--- a/include/uapi/linux/ipc.h
+++ b/include/uapi/linux/ipc.h
@@ -24,6 +24,7 @@ struct ipc_perm
 #define IPC_CREAT  00001000   /* create if key is nonexistent */
 #define IPC_EXCL   00002000   /* fail if key exists */
 #define IPC_NOWAIT 00004000   /* return error on wait */
+#define IPC_PRESET 00040000   /* use key as id */
 
 /* these fields are used by the DIPC package so the kernel as standard
    should avoid using them if possible */
diff --git a/ipc/msg.c b/ipc/msg.c
index 2f272fa..2f44946 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -190,6 +190,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
 
 	msq->q_perm.mode = msgflg & S_IRWXUGO;
 	msq->q_perm.key = key;
+	msq->q_perm.id = (msgflg & IPC_PRESET) ? key : 0;
 
 	msq->q_perm.security = NULL;
 	retval = security_msg_queue_alloc(msq);
@@ -201,7 +202,8 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
 	/*
 	 * ipc_addid() locks msq
 	 */
-	id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni);
+	id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni,
+		       msgflg & IPC_PRESET);
 	if (id < 0) {
 		security_msg_queue_free(msq);
 		ipc_rcu_putref(msq);
diff --git a/ipc/sem.c b/ipc/sem.c
index 58d31f1..10e9085 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -306,6 +306,7 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
 
 	sma->sem_perm.mode = (semflg & S_IRWXUGO);
 	sma->sem_perm.key = key;
+	sma->sem_perm.id = (semflg & IPC_PRESET) ? key : 0;
 
 	sma->sem_perm.security = NULL;
 	retval = security_sem_alloc(sma);
@@ -314,7 +315,8 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
 		return retval;
 	}
 
-	id = ipc_addid(&sem_ids(ns), &sma->sem_perm, ns->sc_semmni);
+	id = ipc_addid(&sem_ids(ns), &sma->sem_perm, ns->sc_semmni,
+		       semflg & IPC_PRESET);
 	if (id < 0) {
 		security_sem_free(sma);
 		ipc_rcu_putref(sma);
diff --git a/ipc/shm.c b/ipc/shm.c
index dff40c9..80b0046 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -480,6 +480,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 
 	shp->shm_perm.key = key;
 	shp->shm_perm.mode = (shmflg & S_IRWXUGO);
+	shp->shm_perm.id = (shmflg & IPC_PRESET) ? key : 0;
 	shp->mlock_user = NULL;
 
 	shp->shm_perm.security = NULL;
@@ -510,7 +511,8 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 	if (IS_ERR(file))
 		goto no_file;
 
-	id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni);
+	id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni,
+		       shmflg & IPC_PRESET);
 	if (id < 0) {
 		error = id;
 		goto no_id;
diff --git a/ipc/util.c b/ipc/util.c
index 72fd078..503946e 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -238,16 +238,22 @@ int ipc_get_maxid(struct ipc_ids *ids)
  *	@ids: IPC identifier set
  *	@new: new IPC permission set
  *	@size: limit for the number of used ids
+ *	@preset: use passed new->id value as desired id
  *
  *	Add an entry 'new' to the IPC ids idr. The permissions object is
  *	initialised and the first free entry is set up and the id assigned
  *	is returned. The 'new' entry is returned in a locked state on success.
  *	On failure the entry is not locked and a negative err-code is returned.
  *
+ *	If 'preset' is set, then passed new->id is desired to be set for new
+ *	segment. And allocated id is equal to passed value, then ipc ids will
+ *	left unchanged and new->seq will be updated to correspond specified id value.
+ *
  *	Called with ipc_ids.rw_mutex held as a writer.
  */
  
-int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
+int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size,
+	      int preset)
 {
 	kuid_t euid;
 	kgid_t egid;
@@ -264,7 +270,8 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 	rcu_read_lock();
 	spin_lock(&new->lock);
 
-	err = idr_get_new(&ids->ipcs_idr, new, &id);
+	err = idr_get_new_above(&ids->ipcs_idr, new,
+				ipcid_to_idx(new->id), &id);
 	if (err) {
 		spin_unlock(&new->lock);
 		rcu_read_unlock();
@@ -277,6 +284,11 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 	new->cuid = new->uid = euid;
 	new->gid = new->cgid = egid;
 
+	if (preset && ipcid_to_idx(new->id) == id) {
+		new->seq = ipcid_to_seq(new->id);
+		return id;
+	}
+
 	new->seq = ids->seq++;
 	if(ids->seq > ids->seq_max)
 		ids->seq = 0;
@@ -736,7 +748,7 @@ struct kern_ipc_perm *ipc_lock_check(struct ipc_ids *ids, int id)
 int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
 			struct ipc_ops *ops, struct ipc_params *params)
 {
-	if (params->key == IPC_PRIVATE)
+	if (params->key == IPC_PRIVATE && ((params->flg & IPC_PRESET) == 0))
 		return ipcget_new(ns, ids, ops, params);
 	else
 		return ipcget_public(ns, ids, ops, params);
diff --git a/ipc/util.h b/ipc/util.h
index c8fe2f7..3a9e558 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -92,9 +92,10 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
 #define IPC_SHM_IDS	2
 
 #define ipcid_to_idx(id) ((id) % SEQ_MULTIPLIER)
+#define ipcid_to_seq(id) ((id) / SEQ_MULTIPLIER)
 
 /* must be called with ids->rw_mutex acquired for writing */
-int ipc_addid(struct ipc_ids *, struct kern_ipc_perm *, int);
+int ipc_addid(struct ipc_ids *, struct kern_ipc_perm *, int, int);
 
 /* must be called with ids->rw_mutex acquired for reading */
 int ipc_get_maxid(struct ipc_ids *);


^ permalink raw reply related

* [PATCH v7 03/10] ipc: segment key change helper introduced
From: Stanislav Kinsbursky @ 2012-10-18 10:22 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

This patch introduces existent segment key changing infrastructure.
New function ipc_update_key() can be used change segment key, cuid, cgid
values. It checks for that new key is not used (except IPC_PRIVATE) prior to
set it on existent.
To make this possible, added copying of this fields from user-space in
__get_compat_ipc_perm() and __get_compat_ipc64_perm() functions. Also segment
search by key and lock were splitted into different functions, because
ipc_update_key() doesn't need to lock the segment during check that new key is
not used.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 ipc/compat.c |    6 ++++++
 ipc/util.c   |   49 ++++++++++++++++++++++++++++++++++++++++++++++---
 ipc/util.h   |    2 ++
 3 files changed, 54 insertions(+), 3 deletions(-)

diff --git a/ipc/compat.c b/ipc/compat.c
index ad9518e..af30d13 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -144,6 +144,9 @@ static inline int __get_compat_ipc64_perm(struct ipc64_perm *p64,
 	err  = __get_user(p64->uid, &up64->uid);
 	err |= __get_user(p64->gid, &up64->gid);
 	err |= __get_user(p64->mode, &up64->mode);
+	err |= __get_user(p64->cuid, &up64->cuid);
+	err |= __get_user(p64->cgid, &up64->cgid);
+	err |= __get_user(p64->key, &up64->key);
 	return err;
 }
 
@@ -155,6 +158,9 @@ static inline int __get_compat_ipc_perm(struct ipc64_perm *p,
 	err  = __get_user(p->uid, &up->uid);
 	err |= __get_user(p->gid, &up->gid);
 	err |= __get_user(p->mode, &up->mode);
+	err |= __get_user(p->cuid, &up->cuid);
+	err |= __get_user(p->cgid, &up->cgid);
+	err |= __get_user(p->key, &up->key);
 	return err;
 }
 
diff --git a/ipc/util.c b/ipc/util.c
index 503946e..3396ab5 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -173,7 +173,7 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
  *	@key: The key to find
  *	
  *	Requires ipc_ids.rw_mutex locked.
- *	Returns the LOCKED pointer to the ipc structure if found or NULL
+ *	Returns the UNLOCKED pointer to the ipc structure if found or NULL
  *	if not.
  *	If key is found ipc points to the owning ipc structure
  */
@@ -195,7 +195,6 @@ static struct kern_ipc_perm *ipc_findkey(struct ipc_ids *ids, key_t key)
 			continue;
 		}
 
-		ipc_lock_by_ptr(ipc);
 		return ipc;
 	}
 
@@ -203,6 +202,27 @@ static struct kern_ipc_perm *ipc_findkey(struct ipc_ids *ids, key_t key)
 }
 
 /**
+ *	ipc_findkey_locked	-	find and lock a key in an ipc identifier set
+ *	@ids: Identifier set
+ *	@key: The key to find
+ *
+ *	Requires ipc_ids.rw_mutex locked.
+ *	Returns the LOCKED pointer to the ipc structure if found or NULL
+ *	if not.
+ *	If key is found ipc points to the owning ipc structure
+ */
+
+static struct kern_ipc_perm *ipc_findkey_locked(struct ipc_ids *ids, key_t key)
+{
+	struct kern_ipc_perm *ipc;
+
+	ipc = ipc_findkey(ids, key);
+	if (ipc)
+		ipc_lock_by_ptr(ipc);
+	return ipc;
+}
+
+/**
  *	ipc_get_maxid 	-	get the last assigned id
  *	@ids: IPC identifier set
  *
@@ -388,7 +408,7 @@ retry:
 	 * a new entry + read locks are not "upgradable"
 	 */
 	down_write(&ids->rw_mutex);
-	ipcp = ipc_findkey(ids, params->key);
+	ipcp = ipc_findkey_locked(ids, params->key);
 	if (ipcp == NULL) {
 		/* key not used */
 		if (!(flg & IPC_CREAT))
@@ -755,6 +775,29 @@ int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
 }
 
 /**
+ * ipc_update_key - update the key of an IPC.
+ * @in:  the permission given as input.
+ * @out: the permission of the ipc to set.
+ *
+ * Common routine called by sys_shmctl(), sys_semctl(). sys_msgctl().
+ */
+int ipc_update_key(struct ipc_ids *ids, struct ipc64_perm *in,
+		    struct kern_ipc_perm *out)
+{
+
+	if (in->key && out->key != in->key) {
+		/*
+		 * Check for existent segment with the same key.
+		 * Note: ipc_ids.rw_mutex is taken for write already.
+		 */
+		if (ipc_findkey(ids, in->key))
+			return -EEXIST;
+		out->key = in->key;
+	}
+	return 0;
+}
+
+/**
  * ipc_update_perm - update the permissions of an IPC.
  * @in:  the permission given as input.
  * @out: the permission of the ipc to set.
diff --git a/ipc/util.h b/ipc/util.h
index 3a9e558..271bded 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -126,6 +126,8 @@ struct kern_ipc_perm *ipc_lock(struct ipc_ids *, int);
 
 void kernel_to_ipc64_perm(struct kern_ipc_perm *in, struct ipc64_perm *out);
 void ipc64_perm_to_ipc_perm(struct ipc64_perm *in, struct ipc_perm *out);
+int ipc_update_key(struct ipc_ids *ids, struct ipc64_perm *in,
+		   struct kern_ipc_perm *out);
 int ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out);
 struct kern_ipc_perm *ipcctl_pre_down(struct ipc_namespace *ns,
 				      struct ipc_ids *ids, int id, int cmd,

^ permalink raw reply related

* [PATCH v7 04/10] ipc: add new SHM_SET command for sys_shmctl() call
From: Stanislav Kinsbursky @ 2012-10-18 10:22 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

New SHM_SET command will be interpreted exactly as IPC_SET, but also will
update key, cuid and cgid values. IOW, it allows to change existent key value.
The fact, that key is not used is checked before update. Otherwise -EEXIST is
returned.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 include/uapi/linux/shm.h   |    1 +
 ipc/compat.c               |    1 +
 ipc/shm.c                  |   14 ++++++++++++--
 security/selinux/hooks.c   |    1 +
 security/smack/smack_lsm.c |    1 +
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/shm.h b/include/uapi/linux/shm.h
index ec36fa1..d7413fd 100644
--- a/include/uapi/linux/shm.h
+++ b/include/uapi/linux/shm.h
@@ -56,6 +56,7 @@ struct shmid_ds {
 /* ipcs ctl commands */
 #define SHM_STAT 	13
 #define SHM_INFO 	14
+#define SHM_SET		15
 
 /* Obsolete, used only for backwards compatibility */
 struct	shminfo {
diff --git a/ipc/compat.c b/ipc/compat.c
index af30d13..35c750d 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -692,6 +692,7 @@ long compat_sys_shmctl(int first, int second, void __user *uptr)
 
 
 	case IPC_SET:
+	case SHM_SET:
 		if (version == IPC_64) {
 			err = get_compat_shmid64_ds(&s64, uptr);
 		} else {
diff --git a/ipc/shm.c b/ipc/shm.c
index 80b0046..650911e 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -636,6 +636,9 @@ copy_shmid_from_user(struct shmid64_ds *out, void __user *buf, int version)
 		out->shm_perm.uid	= tbuf_old.shm_perm.uid;
 		out->shm_perm.gid	= tbuf_old.shm_perm.gid;
 		out->shm_perm.mode	= tbuf_old.shm_perm.mode;
+		out->shm_perm.cuid	= tbuf_old.shm_perm.cuid;
+		out->shm_perm.cgid	= tbuf_old.shm_perm.cgid;
+		out->shm_perm.key	= tbuf_old.shm_perm.key;
 
 		return 0;
 	    }
@@ -740,12 +743,13 @@ static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
 	struct shmid_kernel *shp;
 	int err;
 
-	if (cmd == IPC_SET) {
+	if (cmd == IPC_SET || cmd == SHM_SET) {
 		if (copy_shmid_from_user(&shmid64, buf, version))
 			return -EFAULT;
 	}
 
-	ipcp = ipcctl_pre_down(ns, &shm_ids(ns), shmid, cmd,
+	ipcp = ipcctl_pre_down(ns, &shm_ids(ns), shmid,
+			       (cmd != SHM_SET) ? cmd : IPC_SET,
 			       &shmid64.shm_perm, 0);
 	if (IS_ERR(ipcp))
 		return PTR_ERR(ipcp);
@@ -759,6 +763,11 @@ static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
 	case IPC_RMID:
 		do_shm_rmid(ns, ipcp);
 		goto out_up;
+	case SHM_SET:
+		err = ipc_update_key(&shm_ids(ns), &shmid64.shm_perm, ipcp);
+		if (err)
+			break;
+		 /* fall through */
 	case IPC_SET:
 		err = ipc_update_perm(&shmid64.shm_perm, ipcp);
 		if (err)
@@ -938,6 +947,7 @@ SYSCALL_DEFINE3(shmctl, int, shmid, int, cmd, struct shmid_ds __user *, buf)
 	}
 	case IPC_RMID:
 	case IPC_SET:
+	case SHM_SET:
 		err = shmctl_down(ns, shmid, cmd, buf, version);
 		return err;
 	default:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 24ab414..62b2447 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5027,6 +5027,7 @@ static int selinux_shm_shmctl(struct shmid_kernel *shp, int cmd)
 		perms = SHM__GETATTR | SHM__ASSOCIATE;
 		break;
 	case IPC_SET:
+	case SHM_SET:
 		perms = SHM__SETATTR;
 		break;
 	case SHM_LOCK:
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 38be92c..c7eabc9 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2121,6 +2121,7 @@ static int smack_shm_shmctl(struct shmid_kernel *shp, int cmd)
 		may = MAY_READ;
 		break;
 	case IPC_SET:
+	case SHM_SET:
 	case SHM_LOCK:
 	case SHM_UNLOCK:
 	case IPC_RMID:

^ permalink raw reply related

* [PATCH v7 05/10] ipc: add new MSG_SET command for sys_msgctl() call
From: Stanislav Kinsbursky @ 2012-10-18 10:23 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

New MSG_SET command will be interpreted exactly as IPC_SET, but also will
update key, cuid and cgid values. IOW, it allows to change existent key value.
The fact, that key is not used is checked before update. Otherwise -EEXIST is
returned.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 include/uapi/linux/msg.h   |    1 +
 ipc/compat.c               |    1 +
 ipc/msg.c                  |   14 ++++++++++++--
 security/selinux/hooks.c   |    1 +
 security/smack/smack_lsm.c |    1 +
 5 files changed, 16 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/msg.h b/include/uapi/linux/msg.h
index 78dbd2f..76999c9 100644
--- a/include/uapi/linux/msg.h
+++ b/include/uapi/linux/msg.h
@@ -6,6 +6,7 @@
 /* ipcs ctl commands */
 #define MSG_STAT 11
 #define MSG_INFO 12
+#define MSG_SET  13
 
 /* msgrcv options */
 #define MSG_NOERROR     010000  /* no error if message is too big */
diff --git a/ipc/compat.c b/ipc/compat.c
index 35c750d..9c70f9a 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -483,6 +483,7 @@ long compat_sys_msgctl(int first, int second, void __user *uptr)
 		break;
 
 	case IPC_SET:
+	case MSG_SET:
 		if (version == IPC_64) {
 			err = get_compat_msqid64(&m64, uptr);
 		} else {
diff --git a/ipc/msg.c b/ipc/msg.c
index 2f44946..3c13b99 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -392,6 +392,9 @@ copy_msqid_from_user(struct msqid64_ds *out, void __user *buf, int version)
 		out->msg_perm.uid      	= tbuf_old.msg_perm.uid;
 		out->msg_perm.gid      	= tbuf_old.msg_perm.gid;
 		out->msg_perm.mode     	= tbuf_old.msg_perm.mode;
+		out->msg_perm.cuid	= tbuf_old.msg_perm.cuid;
+		out->msg_perm.cgid	= tbuf_old.msg_perm.cgid;
+		out->msg_perm.key	= tbuf_old.msg_perm.key;
 
 		if (tbuf_old.msg_qbytes == 0)
 			out->msg_qbytes	= tbuf_old.msg_lqbytes;
@@ -418,12 +421,13 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
 	struct msg_queue *msq;
 	int err;
 
-	if (cmd == IPC_SET) {
+	if (cmd == IPC_SET || cmd == MSG_SET) {
 		if (copy_msqid_from_user(&msqid64, buf, version))
 			return -EFAULT;
 	}
 
-	ipcp = ipcctl_pre_down(ns, &msg_ids(ns), msqid, cmd,
+	ipcp = ipcctl_pre_down(ns, &msg_ids(ns), msqid,
+			       (cmd != MSG_SET) ? cmd : IPC_SET,
 			       &msqid64.msg_perm, msqid64.msg_qbytes);
 	if (IS_ERR(ipcp))
 		return PTR_ERR(ipcp);
@@ -438,6 +442,11 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
 	case IPC_RMID:
 		freeque(ns, ipcp);
 		goto out_up;
+	case MSG_SET:
+		err = ipc_update_key(&msg_ids(ns), &msqid64.msg_perm, ipcp);
+		if (err)
+			break;
+		 /* fall through */
 	case IPC_SET:
 		if (msqid64.msg_qbytes > ns->msg_ctlmnb &&
 		    !capable(CAP_SYS_RESOURCE)) {
@@ -569,6 +578,7 @@ SYSCALL_DEFINE3(msgctl, int, msqid, int, cmd, struct msqid_ds __user *, buf)
 	}
 	case IPC_SET:
 	case IPC_RMID:
+	case MSG_SET:
 		err = msgctl_down(ns, msqid, cmd, buf, version);
 		return err;
 	default:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 62b2447..78b77ac 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -4885,6 +4885,7 @@ static int selinux_msg_queue_msgctl(struct msg_queue *msq, int cmd)
 		perms = MSGQ__GETATTR | MSGQ__ASSOCIATE;
 		break;
 	case IPC_SET:
+	case MSG_SET:
 		perms = MSGQ__SETATTR;
 		break;
 	case IPC_RMID:
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index c7eabc9..d51a8da 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2374,6 +2374,7 @@ static int smack_msg_queue_msgctl(struct msg_queue *msq, int cmd)
 		may = MAY_READ;
 		break;
 	case IPC_SET:
+	case MSG_SET:
 	case IPC_RMID:
 		may = MAY_READWRITE;
 		break;

^ permalink raw reply related

* [PATCH v7 06/10] qlge driver: rename internal SEM_SET macro to SEM_INIT
From: Stanislav Kinsbursky @ 2012-10-18 10:23 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

The reason for this patch is that SET_SET is desired to be a new part IPC
sys_semctl() API.
The name itself for IPC is quite natural, because all linux-specific commands
names for IPC system calls are originally created by replacing "IPC_" part by
"SEM_"("MSG_", "SHM_") part.
So, I'm hoping, that this change doesn't really matters for "QLogic qlge NIC
HBA Driver" developers, since it's just an internal define.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 drivers/net/ethernet/qlogic/qlge/qlge.h      |    4 ++--
 drivers/net/ethernet/qlogic/qlge/qlge_main.c |   16 ++++++++--------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/qlogic/qlge/qlge.h b/drivers/net/ethernet/qlogic/qlge/qlge.h
index a131d7b..6f46ea5 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge.h
+++ b/drivers/net/ethernet/qlogic/qlge/qlge.h
@@ -347,10 +347,10 @@ enum {
 enum {
 	/*
 	 * Example:
-	 * reg = SEM_XGMAC0_MASK | (SEM_SET << SEM_XGMAC0_SHIFT)
+	 * reg = SEM_XGMAC0_MASK | (SEM_INIT << SEM_XGMAC0_SHIFT)
 	 */
 	SEM_CLEAR = 0,
-	SEM_SET = 1,
+	SEM_INIT = 1,
 	SEM_FORCE = 3,
 	SEM_XGMAC0_SHIFT = 0,
 	SEM_XGMAC1_SHIFT = 2,
diff --git a/drivers/net/ethernet/qlogic/qlge/qlge_main.c b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
index b262d61..cfb0f62 100644
--- a/drivers/net/ethernet/qlogic/qlge/qlge_main.c
+++ b/drivers/net/ethernet/qlogic/qlge/qlge_main.c
@@ -109,28 +109,28 @@ static int ql_sem_trylock(struct ql_adapter *qdev, u32 sem_mask)
 
 	switch (sem_mask) {
 	case SEM_XGMAC0_MASK:
-		sem_bits = SEM_SET << SEM_XGMAC0_SHIFT;
+		sem_bits = SEM_INIT << SEM_XGMAC0_SHIFT;
 		break;
 	case SEM_XGMAC1_MASK:
-		sem_bits = SEM_SET << SEM_XGMAC1_SHIFT;
+		sem_bits = SEM_INIT << SEM_XGMAC1_SHIFT;
 		break;
 	case SEM_ICB_MASK:
-		sem_bits = SEM_SET << SEM_ICB_SHIFT;
+		sem_bits = SEM_INIT << SEM_ICB_SHIFT;
 		break;
 	case SEM_MAC_ADDR_MASK:
-		sem_bits = SEM_SET << SEM_MAC_ADDR_SHIFT;
+		sem_bits = SEM_INIT << SEM_MAC_ADDR_SHIFT;
 		break;
 	case SEM_FLASH_MASK:
-		sem_bits = SEM_SET << SEM_FLASH_SHIFT;
+		sem_bits = SEM_INIT << SEM_FLASH_SHIFT;
 		break;
 	case SEM_PROBE_MASK:
-		sem_bits = SEM_SET << SEM_PROBE_SHIFT;
+		sem_bits = SEM_INIT << SEM_PROBE_SHIFT;
 		break;
 	case SEM_RT_IDX_MASK:
-		sem_bits = SEM_SET << SEM_RT_IDX_SHIFT;
+		sem_bits = SEM_INIT << SEM_RT_IDX_SHIFT;
 		break;
 	case SEM_PROC_REG_MASK:
-		sem_bits = SEM_SET << SEM_PROC_REG_SHIFT;
+		sem_bits = SEM_INIT << SEM_PROC_REG_SHIFT;
 		break;
 	default:
 		netif_alert(qdev, probe, qdev->ndev, "bad Semaphore mask!.\n");

^ permalink raw reply related

* [PATCH v7 07/10] ipc: add new SEM_SET command for sys_semctl() call
From: Stanislav Kinsbursky @ 2012-10-18 10:23 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

New SEM_SET command will be interpreted exactly as IPC_SET, but also will
update key, cuid and cgid values. IOW, it allows to change existent key value.
The fact, that key is not used is checked before update. Otherwise -EEXIST is
returned.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 include/uapi/linux/sem.h   |    1 +
 ipc/compat.c               |    1 +
 ipc/sem.c                  |   11 +++++++++--
 security/selinux/hooks.c   |    1 +
 security/smack/smack_lsm.c |    1 +
 5 files changed, 13 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/sem.h b/include/uapi/linux/sem.h
index 541fce0..b6ae374 100644
--- a/include/uapi/linux/sem.h
+++ b/include/uapi/linux/sem.h
@@ -18,6 +18,7 @@
 /* ipcs ctl cmds */
 #define SEM_STAT 18
 #define SEM_INFO 19
+#define SEM_SET  20
 
 /* Obsolete, used only for backwards compatibility and libc5 compiles */
 struct semid_ds {
diff --git a/ipc/compat.c b/ipc/compat.c
index 9c70f9a..84d8efd 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -290,6 +290,7 @@ static long do_compat_semctl(int first, int second, int third, u32 pad)
 		break;
 
 	case IPC_SET:
+	case SEM_SET:
 		if (version == IPC_64) {
 			err = get_compat_semid64_ds(&s64, compat_ptr(pad));
 		} else {
diff --git a/ipc/sem.c b/ipc/sem.c
index 10e9085..13b480a 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1085,12 +1085,13 @@ static int semctl_down(struct ipc_namespace *ns, int semid,
 	struct semid64_ds semid64;
 	struct kern_ipc_perm *ipcp;
 
-	if(cmd == IPC_SET) {
+	if (cmd == IPC_SET || cmd == SEM_SET) {
 		if (copy_semid_from_user(&semid64, arg.buf, version))
 			return -EFAULT;
 	}
 
-	ipcp = ipcctl_pre_down(ns, &sem_ids(ns), semid, cmd,
+	ipcp = ipcctl_pre_down(ns, &sem_ids(ns), semid,
+			       (cmd != SEM_SET) ? cmd : IPC_SET,
 			       &semid64.sem_perm, 0);
 	if (IS_ERR(ipcp))
 		return PTR_ERR(ipcp);
@@ -1105,6 +1106,11 @@ static int semctl_down(struct ipc_namespace *ns, int semid,
 	case IPC_RMID:
 		freeary(ns, ipcp);
 		goto out_up;
+	case SEM_SET:
+		err = ipc_update_key(&sem_ids(ns), &semid64.sem_perm, ipcp);
+		if (err)
+			break;
+		 /* fall through */
 	case IPC_SET:
 		err = ipc_update_perm(&semid64.sem_perm, ipcp);
 		if (err)
@@ -1152,6 +1158,7 @@ SYSCALL_DEFINE(semctl)(int semid, int semnum, int cmd, union semun arg)
 		return err;
 	case IPC_RMID:
 	case IPC_SET:
+	case SEM_SET:
 		err = semctl_down(ns, semid, cmd, version, arg);
 		return err;
 	default:
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 78b77ac..02b037d 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -5133,6 +5133,7 @@ static int selinux_sem_semctl(struct sem_array *sma, int cmd)
 		perms = SEM__DESTROY;
 		break;
 	case IPC_SET:
+	case SEM_SET:
 		perms = SEM__SETATTR;
 		break;
 	case IPC_STAT:
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index d51a8da..b4135ed 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -2253,6 +2253,7 @@ static int smack_sem_semctl(struct sem_array *sma, int cmd)
 	case SETALL:
 	case IPC_RMID:
 	case IPC_SET:
+	case SEM_SET:
 		may = MAY_READWRITE;
 		break;
 	case IPC_INFO:

^ permalink raw reply related

* [PATCH v7 08/10] IPC: message queue receive cleanup
From: Stanislav Kinsbursky @ 2012-10-18 10:23 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

This patch moves all message related manipulation into one function msg_fill().
Actually, two functions because of the compat one.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 include/linux/msg.h |    5 +++--
 ipc/compat.c        |   45 +++++++++++++++++++--------------------------
 ipc/msg.c           |   44 +++++++++++++++++++++++---------------------
 3 files changed, 45 insertions(+), 49 deletions(-)

diff --git a/include/linux/msg.h b/include/linux/msg.h
index 7a4b9e9..f38edba 100644
--- a/include/linux/msg.h
+++ b/include/linux/msg.h
@@ -34,7 +34,8 @@ struct msg_queue {
 /* Helper routines for sys_msgsnd and sys_msgrcv */
 extern long do_msgsnd(int msqid, long mtype, void __user *mtext,
 			size_t msgsz, int msgflg);
-extern long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
-			size_t msgsz, long msgtyp, int msgflg);
+extern long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
+		      int msgflg,
+		      long (*msg_fill)(void __user *, struct msg_msg *, size_t ));
 
 #endif /* _LINUX_MSG_H */
diff --git a/ipc/compat.c b/ipc/compat.c
index 84d8efd..1fd8d1c 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -313,6 +313,20 @@ static long do_compat_semctl(int first, int second, int third, u32 pad)
 	return err;
 }
 
+long compat_do_msg_fill(void __user *dest, struct msg_msg *msg, size_t bufsz)
+{
+	struct compat_msgbuf __user *msgp = dest;
+	size_t msgsz;
+
+	if (put_user(msg->m_type, &msgp->mtype))
+		return -EFAULT;
+
+	msgsz = (bufsz > msg->m_ts) ? msg->m_ts : bufsz;
+	if (store_msg(msgp->mtext, msg, msgsz))
+		return -EFAULT;
+	return msgsz;
+}
+
 #ifdef CONFIG_ARCH_WANT_OLD_COMPAT_IPC
 long compat_sys_semctl(int first, int second, int third, void __user *uptr)
 {
@@ -344,10 +358,6 @@ long compat_sys_msgsnd(int first, int second, int third, void __user *uptr)
 long compat_sys_msgrcv(int first, int second, int msgtyp, int third,
 			   int version, void __user *uptr)
 {
-	struct compat_msgbuf __user *up;
-	long type;
-	int err;
-
 	if (first < 0)
 		return -EINVAL;
 	if (second < 0)
@@ -355,23 +365,14 @@ long compat_sys_msgrcv(int first, int second, int msgtyp, int third,
 
 	if (!version) {
 		struct compat_ipc_kludge ipck;
-		err = -EINVAL;
 		if (!uptr)
-			goto out;
-		err = -EFAULT;
+			return -EINVAL;
 		if (copy_from_user (&ipck, uptr, sizeof(ipck)))
-			goto out;
+			return -EFAULT;
 		uptr = compat_ptr(ipck.msgp);
 		msgtyp = ipck.msgtyp;
 	}
-	up = uptr;
-	err = do_msgrcv(first, &type, up->mtext, second, msgtyp, third);
-	if (err < 0)
-		goto out;
-	if (put_user(type, &up->mtype))
-		err = -EFAULT;
-out:
-	return err;
+	return do_msgrcv(first, uptr, second, msgtyp, third, compat_do_msg_fill);
 }
 #else
 long compat_sys_semctl(int semid, int semnum, int cmd, int arg)
@@ -392,16 +393,8 @@ long compat_sys_msgsnd(int msqid, struct compat_msgbuf __user *msgp,
 long compat_sys_msgrcv(int msqid, struct compat_msgbuf __user *msgp,
 		       compat_ssize_t msgsz, long msgtyp, int msgflg)
 {
-	long err, mtype;
-
-	err =  do_msgrcv(msqid, &mtype, msgp->mtext, (ssize_t)msgsz, msgtyp, msgflg);
-	if (err < 0)
-		goto out;
-
-	if (put_user(mtype, &msgp->mtype))
-		err = -EFAULT;
- out:
-	return err;
+	return do_msgrcv(msqid, msgp, (ssize_t)msgsz, msgtyp, msgflg,
+			 compat_do_msg_fill);
 }
 #endif
 
diff --git a/ipc/msg.c b/ipc/msg.c
index 3c13b99..028ab87 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -767,15 +767,30 @@ static inline int convert_mode(long *msgtyp, int msgflg)
 	return SEARCH_EQUAL;
 }
 
-long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
-		size_t msgsz, long msgtyp, int msgflg)
+static long do_msg_fill(void __user *dest, struct msg_msg *msg, size_t bufsz)
+{
+	struct msgbuf __user *msgp = dest;
+	size_t msgsz;
+
+	if (put_user(msg->m_type, &msgp->mtype))
+		return -EFAULT;
+
+	msgsz = (bufsz > msg->m_ts) ? msg->m_ts : bufsz;
+	if (store_msg(msgp->mtext, msg, msgsz))
+		return -EFAULT;
+	return msgsz;
+}
+
+long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
+	       int msgflg,
+	       long (*msg_handler)(void __user *, struct msg_msg *, size_t ))
 {
 	struct msg_queue *msq;
 	struct msg_msg *msg;
 	int mode;
 	struct ipc_namespace *ns;
 
-	if (msqid < 0 || (long) msgsz < 0)
+	if (msqid < 0 || (long) bufsz < 0)
 		return -EINVAL;
 	mode = convert_mode(&msgtyp, msgflg);
 	ns = current->nsproxy->ipc_ns;
@@ -816,7 +831,7 @@ long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
 			 * Found a suitable message.
 			 * Unlink it from the queue.
 			 */
-			if ((msgsz < msg->m_ts) && !(msgflg & MSG_NOERROR)) {
+			if ((bufsz < msg->m_ts) && !(msgflg & MSG_NOERROR)) {
 				msg = ERR_PTR(-E2BIG);
 				goto out_unlock;
 			}
@@ -843,7 +858,7 @@ long do_msgrcv(int msqid, long *pmtype, void __user *mtext,
 		if (msgflg & MSG_NOERROR)
 			msr_d.r_maxsize = INT_MAX;
 		else
-			msr_d.r_maxsize = msgsz;
+			msr_d.r_maxsize = bufsz;
 		msr_d.r_msg = ERR_PTR(-EAGAIN);
 		current->state = TASK_INTERRUPTIBLE;
 		msg_unlock(msq);
@@ -906,29 +921,16 @@ out_unlock:
 	if (IS_ERR(msg))
 		return PTR_ERR(msg);
 
-	msgsz = (msgsz > msg->m_ts) ? msg->m_ts : msgsz;
-	*pmtype = msg->m_type;
-	if (store_msg(mtext, msg, msgsz))
-		msgsz = -EFAULT;
-
+	bufsz = msg_handler(buf, msg, bufsz);
 	free_msg(msg);
 
-	return msgsz;
+	return bufsz;
 }
 
 SYSCALL_DEFINE5(msgrcv, int, msqid, struct msgbuf __user *, msgp, size_t, msgsz,
 		long, msgtyp, int, msgflg)
 {
-	long err, mtype;
-
-	err =  do_msgrcv(msqid, &mtype, msgp->mtext, msgsz, msgtyp, msgflg);
-	if (err < 0)
-		goto out;
-
-	if (put_user(mtype, &msgp->mtype))
-		err = -EFAULT;
-out:
-	return err;
+	return do_msgrcv(msqid, msgp, msgsz, msgtyp, msgflg, do_msg_fill);
 }
 
 #ifdef CONFIG_PROC_FS

^ permalink raw reply related

* [PATCH v7 09/10] IPC: message queue copy feature introduced
From: Stanislav Kinsbursky @ 2012-10-18 10:23 UTC (permalink / raw)
  To: akpm
  Cc: catalin.marinas, will.deacon, dhowells, manfred, hughd, jmorris,
	mtk.manpages, kosaki.motohiro, paulmck, sds, devel, a.p.zijlstra,
	cmetcalf, linux-driver, ron.mercer, viro, eparis, tglx,
	jitendra.kalsaria, netdev, linux-kernel, linux-security-module,
	ebiederm, casey
In-Reply-To: <20121018101543.16036.12221.stgit@localhost.localdomain>

This patch is required for checkpoint/restore in userspace.
IOW, c/r requires some way to get all pending IPC messages without deleting
them from the queue (checkpoint can fail and in this case tasks will be resumed,
so queue have to be valid).
To achive this, new operation flag MSG_COPY for sys_msgrcv() system call was
introduced. If this flag was specified, then mtype is interpreted as number of
the message to copy.
If MSG_COPY is set, then kernel will allocate dummy message with passed size,
and then use new copy_msg() helper function to copy desired message (instead of
unlinking it from the queue).

Notes:
1) Return -ENOSYS if MSG_COPY is specified, but CONFIG_CHECKPOINT_RESTORE is
not set.

Signed-off-by: Stanislav Kinsbursky <skinsbursky@parallels.com>
---
 include/uapi/linux/msg.h |    1 +
 ipc/msg.c                |   50 ++++++++++++++++++++++++++++++++++++++++++++--
 ipc/msgutil.c            |   38 +++++++++++++++++++++++++++++++++++
 ipc/util.h               |    1 +
 4 files changed, 88 insertions(+), 2 deletions(-)

diff --git a/include/uapi/linux/msg.h b/include/uapi/linux/msg.h
index 76999c9..c1af84a 100644
--- a/include/uapi/linux/msg.h
+++ b/include/uapi/linux/msg.h
@@ -11,6 +11,7 @@
 /* msgrcv options */
 #define MSG_NOERROR     010000  /* no error if message is too big */
 #define MSG_EXCEPT      020000  /* recv any msg except of specified type.*/
+#define MSG_COPY        040000  /* copy (not remove) all queue messages */
 
 /* Obsolete, used only for backwards compatibility and libc5 compiles */
 struct msqid_ds {
diff --git a/ipc/msg.c b/ipc/msg.c
index 028ab87..a908529 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -789,19 +789,48 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
 	struct msg_msg *msg;
 	int mode;
 	struct ipc_namespace *ns;
+#ifdef CONFIG_CHECKPOINT_RESTORE
+	struct msg_msg *copy = NULL;
+	unsigned long copy_number = 0;
+#endif
 
 	if (msqid < 0 || (long) bufsz < 0)
 		return -EINVAL;
+	if (msgflg & MSG_COPY) {
+#ifdef CONFIG_CHECKPOINT_RESTORE
+
+		if (msgflg & MSG_COPY) {
+			copy_number = msgtyp;
+			msgtyp = 0;
+		}
+
+		/*
+		 * Create dummy message to copy real message to.
+		 */
+		copy = load_msg(buf, bufsz);
+		if (IS_ERR(copy))
+			return PTR_ERR(copy);
+		copy->m_ts = bufsz;
+#else
+		return -ENOSYS;
+#endif
+	}
 	mode = convert_mode(&msgtyp, msgflg);
 	ns = current->nsproxy->ipc_ns;
 
 	msq = msg_lock_check(ns, msqid);
-	if (IS_ERR(msq))
+	if (IS_ERR(msq)) {
+#ifdef CONFIG_CHECKPOINT_RESTORE
+		if (msgflg & MSG_COPY)
+			free_msg(copy);
+#endif
 		return PTR_ERR(msq);
+	}
 
 	for (;;) {
 		struct msg_receiver msr_d;
 		struct list_head *tmp;
+		long msg_counter = 0;
 
 		msg = ERR_PTR(-EACCES);
 		if (ipcperms(ns, &msq->q_perm, S_IRUGO))
@@ -821,8 +850,16 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
 				if (mode == SEARCH_LESSEQUAL &&
 						walk_msg->m_type != 1) {
 					msgtyp = walk_msg->m_type - 1;
+#ifdef CONFIG_CHECKPOINT_RESTORE
+				} else if (msgflg & MSG_COPY) {
+					if (copy_number == msg_counter) {
+						msg = copy_msg(walk_msg, copy);
+						break;
+					}
+#endif
 				} else
 					break;
+				msg_counter++;
 			}
 			tmp = tmp->next;
 		}
@@ -835,6 +872,10 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
 				msg = ERR_PTR(-E2BIG);
 				goto out_unlock;
 			}
+#ifdef CONFIG_CHECKPOINT_RESTORE
+			if (msgflg & MSG_COPY)
+				goto out_unlock;
+#endif
 			list_del(&msg->m_list);
 			msq->q_qnum--;
 			msq->q_rtime = get_seconds();
@@ -918,8 +959,13 @@ out_unlock:
 			break;
 		}
 	}
-	if (IS_ERR(msg))
+	if (IS_ERR(msg)) {
+#ifdef CONFIG_CHECKPOINT_RESTORE
+		if (msgflg & MSG_COPY)
+			free_msg(copy);
+#endif
 		return PTR_ERR(msg);
+	}
 
 	bufsz = msg_handler(buf, msg, bufsz);
 	free_msg(msg);
diff --git a/ipc/msgutil.c b/ipc/msgutil.c
index 26143d3..b281f5c 100644
--- a/ipc/msgutil.c
+++ b/ipc/msgutil.c
@@ -100,7 +100,45 @@ out_err:
 	free_msg(msg);
 	return ERR_PTR(err);
 }
+#ifdef CONFIG_CHECKPOINT_RESTORE
+struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst)
+{
+	struct msg_msgseg *dst_pseg, *src_pseg;
+	int len = src->m_ts;
+	int alen;
+
+	BUG_ON(dst == NULL);
+	if (src->m_ts > dst->m_ts)
+		return ERR_PTR(-EINVAL);
+
+	alen = len;
+	if (alen > DATALEN_MSG)
+		alen = DATALEN_MSG;
+
+	dst->next = NULL;
+	dst->security = NULL;
 
+	memcpy(dst + 1, src + 1, alen);
+
+	len -= alen;
+	dst_pseg = dst->next;
+	src_pseg = src->next;
+	while (len > 0) {
+		alen = len;
+		if (alen > DATALEN_SEG)
+			alen = DATALEN_SEG;
+		memcpy(dst_pseg + 1, src_pseg + 1, alen);
+		dst_pseg = dst_pseg->next;
+		len -= alen;
+		src_pseg = src_pseg->next;
+	}
+
+	dst->m_type = src->m_type;
+	dst->m_ts = src->m_ts;
+
+	return dst;
+}
+#endif
 int store_msg(void __user *dest, struct msg_msg *msg, int len)
 {
 	int alen;
diff --git a/ipc/util.h b/ipc/util.h
index 271bded..027f507 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -142,6 +142,7 @@ int ipc_parse_version (int *cmd);
 
 extern void free_msg(struct msg_msg *msg);
 extern struct msg_msg *load_msg(const void __user *src, int len);
+extern struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst);
 extern int store_msg(void __user *dest, struct msg_msg *msg, int len);
 
 extern void recompute_msgmni(struct ipc_namespace *);

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox