Netdev List
 help / color / mirror / Atom feed
* [PATCH 13/17] can: m_can: fix possible sleep in napi poll
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Dong Aisheng, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>

From: Dong Aisheng <b29396@freescale.com>

The m_can_get_berr_counter function can sleep and it may be called in napi poll
function. Rework it to fix the following warning.

root@imx6qdlsolo:~# cangen can0 -f -L 12 -D 112233445566778899001122
[ 1846.017565] m_can 20e8000.can can0: entered error warning state
[ 1846.023551] ------------[ cut here ]------------
[ 1846.028216] WARNING: CPU: 0 PID: 560 at kernel/locking/mutex.c:867 mutex_trylock+0x218/0x23c()
[ 1846.036889] DEBUG_LOCKS_WARN_ON(in_interrupt())
[ 1846.041263] Modules linked in:
[ 1846.044594] CPU: 0 PID: 560 Comm: cangen Not tainted 3.17.0-rc4-next-20140915-00010-g032d018-dirty #477
[ 1846.054033] Backtrace:
[ 1846.056557] [<80012448>] (dump_backtrace) from [<80012728>] (show_stack+0x18/0x1c)
[ 1846.064180]  r6:809a07ec r5:809a07ec r4:00000000 r3:00000000
[ 1846.069966] [<80012710>] (show_stack) from [<806c9ee0>] (dump_stack+0x8c/0xa4)
[ 1846.077264] [<806c9e54>] (dump_stack) from [<8002aa78>] (warn_slowpath_common+0x70/0x94)
[ 1846.085403]  r6:806cd1b0 r5:00000009 r4:be1d5c20 r3:be07b0c0
[ 1846.091204] [<8002aa08>] (warn_slowpath_common) from [<8002aad4>] (warn_slowpath_fmt+0x38/0x40)
[ 1846.099951]  r8:8119106c r7:80515aa4 r6:be027000 r5:00000001 r4:809d1df4
[ 1846.106830] [<8002aaa0>] (warn_slowpath_fmt) from [<806cd1b0>] (mutex_trylock+0x218/0x23c)
[ 1846.115141]  r3:80851c88 r2:8084fb74
[ 1846.118804] [<806ccf98>] (mutex_trylock) from [<80515aa4>] (clk_prepare_lock+0x14/0xf4)
[ 1846.126859]  r8:00000040 r7:be1d5cec r6:be027000 r5:be255800 r4:be027000
[ 1846.133737] [<80515a90>] (clk_prepare_lock) from [<80517660>] (clk_prepare+0x14/0x2c)
[ 1846.141583]  r5:be255800 r4:be027000
[ 1846.145272] [<8051764c>] (clk_prepare) from [<8041ff14>] (m_can_get_berr_counter+0x20/0xd4)
[ 1846.153672]  r4:be255800 r3:be07b0c0
[ 1846.157325] [<8041fef4>] (m_can_get_berr_counter) from [<80420428>] (m_can_poll+0x310/0x8fc)
[ 1846.165809]  r7:bd4dc540 r6:00000744 r5:11300000 r4:be255800
[ 1846.171590] [<80420118>] (m_can_poll) from [<8056a468>] (net_rx_action+0xcc/0x1b4)
[ 1846.179204]  r10:00000101 r9:be255ebc r8:00000040 r7:be7c3208 r6:8097c100 r5:be7c3200
[ 1846.187192]  r4:0000012c
[ 1846.189779] [<8056a39c>] (net_rx_action) from [<8002deec>] (__do_softirq+0xfc/0x2c4)
[ 1846.197568]  r10:00000101 r9:8097c088 r8:00000003 r7:8097c080 r6:40000001 r5:8097c08c
[ 1846.205559]  r4:00000020
[ 1846.208144] [<8002ddf0>] (__do_softirq) from [<8002e194>] (do_softirq+0x7c/0x88)
[ 1846.215588]  r10:00000000 r9:bd516a60 r8:be18ce00 r7:00000000 r6:be255800 r5:8056c0ec
[ 1846.223578]  r4:60000093
[ 1846.226163] [<8002e118>] (do_softirq) from [<8002e288>] (__local_bh_enable_ip+0xe8/0x10c)
[ 1846.234386]  r4:00000200 r3:be1d4000
[ 1846.238036] [<8002e1a0>] (__local_bh_enable_ip) from [<8056c108>] (__dev_queue_xmit+0x314/0x6b0)
[ 1846.246868]  r6:be255800 r5:bd516a00 r4:00000000 r3:be07b0c0
[ 1846.252645] [<8056bdf4>] (__dev_queue_xmit) from [<8056c4b8>] (dev_queue_xmit+0x14/0x18)

Signed-off-by: Dong Aisheng <b29396@freescale.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 268ad5064c47..214160b4c314 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -481,11 +481,23 @@ static int m_can_handle_lec_err(struct net_device *dev,
 	return 1;
 }
 
+static int __m_can_get_berr_counter(const struct net_device *dev,
+				    struct can_berr_counter *bec)
+{
+	struct m_can_priv *priv = netdev_priv(dev);
+	unsigned int ecr;
+
+	ecr = m_can_read(priv, M_CAN_ECR);
+	bec->rxerr = (ecr & ECR_REC_MASK) >> ECR_REC_SHIFT;
+	bec->txerr = ecr & ECR_TEC_MASK;
+
+	return 0;
+}
+
 static int m_can_get_berr_counter(const struct net_device *dev,
 				  struct can_berr_counter *bec)
 {
 	struct m_can_priv *priv = netdev_priv(dev);
-	unsigned int ecr;
 	int err;
 
 	err = clk_prepare_enable(priv->hclk);
@@ -498,9 +510,7 @@ static int m_can_get_berr_counter(const struct net_device *dev,
 		return err;
 	}
 
-	ecr = m_can_read(priv, M_CAN_ECR);
-	bec->rxerr = (ecr & ECR_REC_MASK) >> ECR_REC_SHIFT;
-	bec->txerr = ecr & ECR_TEC_MASK;
+	__m_can_get_berr_counter(dev, bec);
 
 	clk_disable_unprepare(priv->cclk);
 	clk_disable_unprepare(priv->hclk);
@@ -544,7 +554,7 @@ static int m_can_handle_state_change(struct net_device *dev,
 	if (unlikely(!skb))
 		return 0;
 
-	m_can_get_berr_counter(dev, &bec);
+	__m_can_get_berr_counter(dev, &bec);
 
 	switch (new_state) {
 	case CAN_STATE_ERROR_ACTIVE:
-- 
2.1.3


^ permalink raw reply related

* [PATCH 12/17] can: m_can: add missing message RAM initialization
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Dong Aisheng, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>

From: Dong Aisheng <b29396@freescale.com>

The M_CAN message RAM is usually equipped with a parity or ECC functionality.
But RAM cells suffer a hardware reset and can therefore hold arbitrary content
at startup - including parity and/or ECC bits.

To prevent the M_CAN controller detecting checksum errors when reading
potentially uninitialized TX message RAM content to transmit CAN frames the TX
message RAM has to be written with (any kind of) initial data.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index e61886bf1bb3..268ad5064c47 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -1010,7 +1010,7 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
 	struct resource *res;
 	void __iomem *addr;
 	u32 out_val[MRAM_CFG_LEN];
-	int ret;
+	int i, start, end, ret;
 
 	/* message ram could be shared */
 	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "message_ram");
@@ -1061,6 +1061,15 @@ static int m_can_of_parse_mram(struct platform_device *pdev,
 		priv->mcfg[MRAM_TXE].off, priv->mcfg[MRAM_TXE].num,
 		priv->mcfg[MRAM_TXB].off, priv->mcfg[MRAM_TXB].num);
 
+	/* initialize the entire Message RAM in use to avoid possible
+	 * ECC/parity checksum errors when reading an uninitialized buffer
+	 */
+	start = priv->mcfg[MRAM_SIDF].off;
+	end = priv->mcfg[MRAM_TXB].off +
+		priv->mcfg[MRAM_TXB].num * TXB_ELEMENT_SIZE;
+	for (i = start; i < end; i += 4)
+		writel(0x0, priv->mram_base + i);
+
 	return 0;
 }
 
-- 
2.1.3


^ permalink raw reply related

* [PATCH 11/17] can: m_can: add CONFIG_HAS_IOMEM dependence
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, David Cohen, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>

From: David Cohen <david.a.cohen@linux.intel.com>

m_can uses io memory which makes it not compilable on architectures
without HAS_IOMEM such as UML:

drivers/built-in.o: In function `m_can_plat_probe':
m_can.c:(.text+0x218cc5): undefined reference to `devm_ioremap_resource'
m_can.c:(.text+0x218df9): undefined reference to `devm_ioremap'

Signed-off-by: David Cohen <david.a.cohen@linux.intel.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/m_can/Kconfig b/drivers/net/can/m_can/Kconfig
index fca5482c09ac..04f20dd39007 100644
--- a/drivers/net/can/m_can/Kconfig
+++ b/drivers/net/can/m_can/Kconfig
@@ -1,4 +1,5 @@
 config CAN_M_CAN
+	depends on HAS_IOMEM
 	tristate "Bosch M_CAN devices"
 	---help---
 	  Say Y here if you want to support for Bosch M_CAN controller.
-- 
2.1.3


^ permalink raw reply related

* [PATCH 10/17] can: m_can: add .ndo_change_mtu function
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Dong Aisheng, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>

From: Dong Aisheng <b29396@freescale.com>

Use common can_change_mtu function.

Signed-off-by: Dong Aisheng <b29396@freescale.com>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/m_can/m_can.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/net/can/m_can/m_can.c b/drivers/net/can/m_can/m_can.c
index 10d571eaed85..e61886bf1bb3 100644
--- a/drivers/net/can/m_can/m_can.c
+++ b/drivers/net/can/m_can/m_can.c
@@ -992,6 +992,7 @@ static const struct net_device_ops m_can_netdev_ops = {
 	.ndo_open = m_can_open,
 	.ndo_stop = m_can_close,
 	.ndo_start_xmit = m_can_start_xmit,
+	.ndo_change_mtu = can_change_mtu,
 };
 
 static int register_m_can_dev(struct net_device *dev)
-- 
2.1.3


^ permalink raw reply related

* [PATCH 05/17] can: remove unused variable
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
  To: netdev
  Cc: davem, linux-can, kernel, Sudip Mukherjee, Sudip Mukherjee,
	Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>

From: Sudip Mukherjee <sudipm.mukherjee@gmail.com>

these variable were only assigned some values, but then never
reused again.
so they are safe to be removed.

Signed-off-by: Sudip Mukherjee <sudip@vectorindia.org>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/sja1000/kvaser_pci.c | 5 +----
 drivers/net/can/usb/ems_usb.c        | 3 +--
 drivers/net/can/usb/esd_usb2.c       | 2 --
 3 files changed, 2 insertions(+), 8 deletions(-)

diff --git a/drivers/net/can/sja1000/kvaser_pci.c b/drivers/net/can/sja1000/kvaser_pci.c
index 8ff3424d5147..15c00faeec61 100644
--- a/drivers/net/can/sja1000/kvaser_pci.c
+++ b/drivers/net/can/sja1000/kvaser_pci.c
@@ -214,7 +214,7 @@ static int kvaser_pci_add_chan(struct pci_dev *pdev, int channel,
 	struct net_device *dev;
 	struct sja1000_priv *priv;
 	struct kvaser_pci *board;
-	int err, init_step;
+	int err;
 
 	dev = alloc_sja1000dev(sizeof(struct kvaser_pci));
 	if (dev == NULL)
@@ -235,7 +235,6 @@ static int kvaser_pci_add_chan(struct pci_dev *pdev, int channel,
 	if (channel == 0) {
 		board->xilinx_ver =
 			ioread8(board->res_addr + XILINX_VERINT) >> 4;
-		init_step = 2;
 
 		/* Assert PTADR# - we're in passive mode so the other bits are
 		   not important */
@@ -264,8 +263,6 @@ static int kvaser_pci_add_chan(struct pci_dev *pdev, int channel,
 	priv->irq_flags = IRQF_SHARED;
 	dev->irq = pdev->irq;
 
-	init_step = 4;
-
 	dev_info(&pdev->dev, "reg_base=%p conf_addr=%p irq=%d\n",
 		 priv->reg_base, board->conf_addr, dev->irq);
 
diff --git a/drivers/net/can/usb/ems_usb.c b/drivers/net/can/usb/ems_usb.c
index 00f2534dde73..29d3f0938eb8 100644
--- a/drivers/net/can/usb/ems_usb.c
+++ b/drivers/net/can/usb/ems_usb.c
@@ -434,10 +434,9 @@ static void ems_usb_read_bulk_callback(struct urb *urb)
 	if (urb->actual_length > CPC_HEADER_SIZE) {
 		struct ems_cpc_msg *msg;
 		u8 *ibuf = urb->transfer_buffer;
-		u8 msg_count, again, start;
+		u8 msg_count, start;
 
 		msg_count = ibuf[0] & ~0x80;
-		again = ibuf[0] & 0x80;
 
 		start = CPC_HEADER_SIZE;
 
diff --git a/drivers/net/can/usb/esd_usb2.c b/drivers/net/can/usb/esd_usb2.c
index 7a90075529c3..c063a54ab8dd 100644
--- a/drivers/net/can/usb/esd_usb2.c
+++ b/drivers/net/can/usb/esd_usb2.c
@@ -464,7 +464,6 @@ static void esd_usb2_write_bulk_callback(struct urb *urb)
 {
 	struct esd_tx_urb_context *context = urb->context;
 	struct esd_usb2_net_priv *priv;
-	struct esd_usb2 *dev;
 	struct net_device *netdev;
 	size_t size = sizeof(struct esd_usb2_msg);
 
@@ -472,7 +471,6 @@ static void esd_usb2_write_bulk_callback(struct urb *urb)
 
 	priv = context->priv;
 	netdev = priv->netdev;
-	dev = priv->usb2;
 
 	/* free up our allocated buffer */
 	usb_free_coherent(urb->dev, size,
-- 
2.1.3


^ permalink raw reply related

* [PATCH 02/17] can: dev: fix typo CIA -> CiA, CAN in Automation
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel, Roman Fietze, Marc Kleine-Budde
In-Reply-To: <1416343067-9810-1-git-send-email-mkl@pengutronix.de>

From: Roman Fietze <roman.fietze@telemotive.de>

This patch fixes a typo in CAN's dev.c:

    CIA -> CiA

which stands for CAN in Automation.

Signed-off-by: Roman Fietze <roman.fietze@telemotive.de>
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
 drivers/net/can/dev.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/net/can/dev.c b/drivers/net/can/dev.c
index 509d5e063d8c..2cfe5012e4e5 100644
--- a/drivers/net/can/dev.c
+++ b/drivers/net/can/dev.c
@@ -110,7 +110,7 @@ static int can_calc_bittiming(struct net_device *dev, struct can_bittiming *bt,
 	long rate;
 	u64 v64;
 
-	/* Use CIA recommended sample points */
+	/* Use CiA recommended sample points */
 	if (bt->sample_point) {
 		sampl_pt = bt->sample_point;
 	} else {
-- 
2.1.3


^ permalink raw reply related

* pull-request: can 2014-11-18
From: Marc Kleine-Budde @ 2014-11-18 20:37 UTC (permalink / raw)
  To: netdev; +Cc: davem, linux-can, kernel

Hello David,

this is a pull request of 17 patches for net/master for the v3.18 release
cycle.

The last patch of this pull request ("can: m_can: update to support CAN FD
features") adds, as the description says, a new feature to the m_can driver. As
the m_can driver has been added in v3.18 there is no risk of causing a
regression. Give me a note if this is not okay and I'll create a new pull
request without it.

There is a patch for the CAN infrastructure by Thomas Körper which fixes
calling kfree_skb() from interrupt context. Roman Fietze fixes a typo also in
the infrastructure. A patch by Dong Aisheng adds a generic helper function to
tell if a skb is normal CAN or CAN-FD frame. Alexey Khoroshilov of the Linux
Driver Verification project fixes a memory leak in the esd_usb2 driver. Two
patches by Sudip Mukherjee remove unused variables and fixe the signess of a
variable. Three patches by me add the missing .ndo_change_mtu callback to the
xilinx_can, rcar_can and gs_usb driver.

The remaining patches improve the m_can driver: David Cohen adds the missing
CONFIG_HAS_IOMEM dependency. Dong Aisheng provides 6 bugfix patches (most
important: missing RAM init, sleep in NAPI poll, dlc in RTR). While the last of
his patches adds CAN FD support to the driver.

regards,
Marc

---

The following changes since commit feb91a02ccb09661507f170b2a444aec94f307f9:

  ipv6: mld: fix add_grhead skb_over_panic for devs with large MTUs (2014-11-16 16:55:06 -0500)

are available in the git repository at:

  git://gitorious.org/linux-can/linux-can.git tags/linux-can-fixes-for-3.18-20141118

for you to fetch changes up to 80646733f11c2e9de3b6339f7e635047e6087280:

  can: m_can: update to support CAN FD features (2014-11-18 21:35:06 +0100)

----------------------------------------------------------------
linux-can-fixes-for-3.18-20141118

----------------------------------------------------------------
Alexey Khoroshilov (1):
      can: esd_usb2: fix memory leak on disconnect

David Cohen (1):
      can: m_can: add CONFIG_HAS_IOMEM dependence

Dong Aisheng (8):
      can: dev: add can_is_canfd_skb() API
      can: m_can: add .ndo_change_mtu function
      can: m_can: add missing message RAM initialization
      can: m_can: fix possible sleep in napi poll
      can: m_can: fix not set can_dlc for remote frame
      can: m_can: add missing delay after setting CCCR_INIT bit
      can: m_can: fix incorrect error messages
      can: m_can: update to support CAN FD features

Marc Kleine-Budde (3):
      can: xilinx_can: add .ndo_change_mtu function
      can: rcar_can: add .ndo_change_mtu function
      can: gs_usb: add .ndo_change_mtu function

Roman Fietze (1):
      can: dev: fix typo CIA -> CiA, CAN in Automation

Sudip Mukherjee (2):
      can: remove unused variable
      can: xilinx_can: fix comparison of unsigned variable

Thomas Körper (1):
      can: dev: avoid calling kfree_skb() from interrupt context

 drivers/net/can/dev.c                |   4 +-
 drivers/net/can/m_can/Kconfig        |   1 +
 drivers/net/can/m_can/m_can.c        | 219 ++++++++++++++++++++++++++---------
 drivers/net/can/rcar_can.c           |   1 +
 drivers/net/can/sja1000/kvaser_pci.c |   5 +-
 drivers/net/can/usb/ems_usb.c        |   3 +-
 drivers/net/can/usb/esd_usb2.c       |   3 +-
 drivers/net/can/usb/gs_usb.c         |   1 +
 drivers/net/can/xilinx_can.c         |   4 +-
 include/linux/can/dev.h              |   6 +
 10 files changed, 183 insertions(+), 64 deletions(-)


^ permalink raw reply

* Re: [PATCH net-next 1/4] net: allow large number of rx queues
From: Cong Wang @ 2014-11-18 20:29 UTC (permalink / raw)
  To: Pankaj Gupta
  Cc: linux-kernel@vger.kernel.org, netdev, David Miller, Jason Wang,
	Michael S. Tsirkin, dgibson, vfalico, Eric Dumazet,
	Vladislav Yasevich, Jerry Chu, wuzhy, Pavel Emelyanov,
	Tom Herbert, bhutchings, xii, Stephen Hemminger,
	Jiří Pírko, sergei.shtylyov
In-Reply-To: <1416327778-17716-2-git-send-email-pagupta@redhat.com>

On Tue, Nov 18, 2014 at 8:22 AM, Pankaj Gupta <pagupta@redhat.com> wrote:
>
> As vmalloc() adds overhead on a critical network path,
> __GFP_REPEAT flag is used with kzalloc() to do this fallback
> only when really needed.
>

Are you sure we need __GFP_REPEAT? We have vmalloc() as
fallback of kmalloc() in many places of networking (grep kvfree),
none of those I checked has this flag set.

^ permalink raw reply

* [PATCH 1/1] net: sched: Deletion of an unnecessary check before the function call "kfree"
From: SF Markus Elfring @ 2014-11-18 20:26 UTC (permalink / raw)
  To: David S. Miller, Jamal Hadi Salim, netdev
  Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 18 Nov 2014 21:21:16 +0100

The kfree() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/sched/cls_bpf.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/sched/cls_bpf.c b/net/sched/cls_bpf.c
index 0e30d58..f323944 100644
--- a/net/sched/cls_bpf.c
+++ b/net/sched/cls_bpf.c
@@ -212,8 +212,7 @@ static int cls_bpf_modify_existing(struct net *net, struct tcf_proto *tp,
 
 	if (fp_old)
 		bpf_prog_destroy(fp_old);
-	if (bpf_old)
-		kfree(bpf_old);
+	kfree(bpf_old);
 
 	return 0;
 
-- 
2.1.3

^ permalink raw reply related

* RE: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
From: Tantilov, Emil S @ 2014-11-18 20:24 UTC (permalink / raw)
  To: Vladislav Yasevich, netdev@vger.kernel.org
  Cc: Kirsher, Jeffrey T, Keller, Jacob E, Skidmore, Donald C
In-Reply-To: <1416338897-13007-1-git-send-email-vyasevic@redhat.com>

>-----Original Message-----
>From: netdev-owner@vger.kernel.org [mailto:netdev-
>owner@vger.kernel.org] On Behalf Of Vladislav Yasevich
>Sent: Tuesday, November 18, 2014 11:28 AM
>To: netdev@vger.kernel.org
>Cc: Vladislav Yasevich; Kirsher, Jeffrey T; Keller, Jacob E
>Subject: [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
>
>IXGBE adapater seems to require that vlan filtering be enabled if VMDQ
>or SRIOV are enabled.  When those functions are disabled,
>vlan filtering may be disabled in promiscuous mode.
>
>Prior to commit a9b8943ee129e11045862d6d6e25c5b63c95403c
>    ixgbe: remove vlan_filter_disable and enable functions
>
>the logic was correct.  However, after the commit the logic
>got reversed and vlan filtered in now turned on when VMDQ/SRIOV
>is disabled.
>
>This patch changes the condition to enable hw vlan filtered
>when VMDQ or SRIOV is enabled.
>
>Fixes: a9b8943ee129e11045862d6d6e25c5b63c95403c (ixgbe:
>remove
>vlan_filter_disable and enable functions)
>CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>CC: Jacob Keller <jacob.e.keller@intel.com>
>Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
>---
> drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
>diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>index d2df4e3..3f81c7a 100644
>--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
>@@ -3936,8 +3936,8 @@ void ixgbe_set_rx_mode(struct
>net_device *netdev)
> 		 * if SR-IOV and VMDQ are disabled - otherwise ensure
> 		 * that hardware VLAN filters remain enabled.
> 		 */
>-		if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
>-					IXGBE_FLAG_SRIOV_ENABLED)))
>+		if (adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
>+				      IXGBE_FLAG_SRIOV_ENABLED))
> 			vlnctrl |= (IXGBE_VLNCTRL_VFE |
>IXGBE_VLNCTRL_CFIEN);
> 	} else {
> 		if (netdev->flags & IFF_ALLMULTI) {

The current logic is correct and it's like this on purpose as it should be obvious by the comment preceding this check. The reason for not disabling the vlan filters in promisc mode is because this can break VLAN isolation between VMs which is considered a security risk.

There is another patch that was proposed:
http://marc.info/?l=linux-netdev&m=141586938625969

which should do what you want using an ethtool toggle.

Thanks,
Emil

^ permalink raw reply

* Re: Question about Patch Submissions
From: Cong Wang @ 2014-11-18 20:21 UTC (permalink / raw)
  To: rapier; +Cc: netdev
In-Reply-To: <546B9A45.9040509@psc.edu>

On Tue, Nov 18, 2014 at 11:13 AM, rapier <rapier@psc.edu> wrote:
> I've read the documentation I've been able to find on the patch submission
> process. I was hoping to clarify one thing though. When submitting a patch
> that spans multiple files and functions is it better to break the submission
> down by function or by file?
>

It depends on what you change, if your patch is trivial, say just cleaning up
whitespaces, or a big single patch doesn't harm review, it is okay not
to split it.
Otherwise, probably you need to split it logically, each of which needs to at
least compile.

^ permalink raw reply

* Question about Patch Submissions
From: rapier @ 2014-11-18 19:13 UTC (permalink / raw)
  To: netdev

I've read the documentation I've been able to find on the patch 
submission process. I was hoping to clarify one thing though. When 
submitting a patch that spans multiple files and functions is it better 
to break the submission down by function or by file?

Thanks for your time,

Chris

^ permalink raw reply

* [PATCH 1/1] netlink: Deletion of an unnecessary check before the function call "__module_get"
From: SF Markus Elfring @ 2014-11-18 20:08 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 18 Nov 2014 21:03:13 +0100

The __module_get() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/netlink/af_netlink.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/netlink/af_netlink.c b/net/netlink/af_netlink.c
index f1de72d..0317b91 100644
--- a/net/netlink/af_netlink.c
+++ b/net/netlink/af_netlink.c
@@ -142,8 +142,7 @@ int netlink_add_tap(struct netlink_tap *nt)
 	list_add_rcu(&nt->list, &netlink_tap_all);
 	spin_unlock(&netlink_tap_lock);
 
-	if (nt->module)
-		__module_get(nt->module);
+	__module_get(nt->module);
 
 	return 0;
 }
-- 
2.1.3

^ permalink raw reply related

* Re: [PATCH net-next] tun: return NET_XMIT_DROP for dropped packets
From: Cong Wang @ 2014-11-18 19:53 UTC (permalink / raw)
  To: Jason Wang
  Cc: David Miller, netdev, linux-kernel@vger.kernel.org,
	Michael S. Tsirkin
In-Reply-To: <1416288041-30921-1-git-send-email-jasowang@redhat.com>

On Mon, Nov 17, 2014 at 9:20 PM, Jason Wang <jasowang@redhat.com> wrote:
> After commit 5d097109257c03a71845729f8db6b5770c4bbedc
> ("tun: only queue packets on device"), NETDEV_TX_OK was returned for
> dropped packets. This will confuse pktgen since dropped packets were
> counted as sent ones.
>
> Fixing this by returning NET_XMIT_DROP to let pktgen count it as error
> packet.

pktgen is suspicious, it sends out packets directly without going through
qdisc, so it should not care about NET_XMIT_* qdisc error code?

Looks like NETDEV_TX_OK doesn't have to mean TX is successful,
the comment says driver takes care of the packet, can be either dropped
or sent out. We might need a new code to distinguish success or failure.

^ permalink raw reply

* [PATCH 1/1] netfilter: Deletion of unnecessary checks before two function calls
From: SF Markus Elfring @ 2014-11-18 19:47 UTC (permalink / raw)
  To: David S. Miller, Jozsef Kadlecsik, Julian Anastasov,
	Pablo Neira Ayuso, Patrick McHardy, Simon Horman, Wensong Zhang,
	netdev, lvs-devel, netfilter-devel, coreteam
  Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 18 Nov 2014 20:37:05 +0100

The functions free_percpu() and module_put() test whether their argument
is NULL and then return immediately. Thus the test around the call is
not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/netfilter/ipvs/ip_vs_ctl.c   | 3 +--
 net/netfilter/ipvs/ip_vs_pe.c    | 3 +--
 net/netfilter/ipvs/ip_vs_sched.c | 3 +--
 net/netfilter/ipvs/ip_vs_sync.c  | 3 +--
 net/netfilter/nf_tables_api.c    | 3 +--
 5 files changed, 5 insertions(+), 10 deletions(-)

diff --git a/net/netfilter/ipvs/ip_vs_ctl.c b/net/netfilter/ipvs/ip_vs_ctl.c
index fd3f444..7c5e40a 100644
--- a/net/netfilter/ipvs/ip_vs_ctl.c
+++ b/net/netfilter/ipvs/ip_vs_ctl.c
@@ -465,8 +465,7 @@ __ip_vs_bind_svc(struct ip_vs_dest *dest, struct ip_vs_service *svc)
 
 static void ip_vs_service_free(struct ip_vs_service *svc)
 {
-	if (svc->stats.cpustats)
-		free_percpu(svc->stats.cpustats);
+	free_percpu(svc->stats.cpustats);
 	kfree(svc);
 }
 
diff --git a/net/netfilter/ipvs/ip_vs_pe.c b/net/netfilter/ipvs/ip_vs_pe.c
index 1a82b29..0df17ca 100644
--- a/net/netfilter/ipvs/ip_vs_pe.c
+++ b/net/netfilter/ipvs/ip_vs_pe.c
@@ -37,8 +37,7 @@ struct ip_vs_pe *__ip_vs_pe_getbyname(const char *pe_name)
 			rcu_read_unlock();
 			return pe;
 		}
-		if (pe->module)
-			module_put(pe->module);
+		module_put(pe->module);
 	}
 	rcu_read_unlock();
 
diff --git a/net/netfilter/ipvs/ip_vs_sched.c b/net/netfilter/ipvs/ip_vs_sched.c
index 4dbcda6..199760c 100644
--- a/net/netfilter/ipvs/ip_vs_sched.c
+++ b/net/netfilter/ipvs/ip_vs_sched.c
@@ -104,8 +104,7 @@ static struct ip_vs_scheduler *ip_vs_sched_getbyname(const char *sched_name)
 			mutex_unlock(&ip_vs_sched_mutex);
 			return sched;
 		}
-		if (sched->module)
-			module_put(sched->module);
+		module_put(sched->module);
 	}
 
 	mutex_unlock(&ip_vs_sched_mutex);
diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
index eadffb2..cafe28d 100644
--- a/net/netfilter/ipvs/ip_vs_sync.c
+++ b/net/netfilter/ipvs/ip_vs_sync.c
@@ -820,8 +820,7 @@ ip_vs_conn_fill_param_sync(struct net *net, int af, union ip_vs_sync_conn *sc,
 
 		p->pe_data = kmemdup(pe_data, pe_data_len, GFP_ATOMIC);
 		if (!p->pe_data) {
-			if (p->pe->module)
-				module_put(p->pe->module);
+			module_put(p->pe->module);
 			return -ENOMEM;
 		}
 		p->pe_data_len = pe_data_len;
diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
index deeb95f..b115f54 100644
--- a/net/netfilter/nf_tables_api.c
+++ b/net/netfilter/nf_tables_api.c
@@ -3472,8 +3472,7 @@ static int nf_tables_abort(struct sk_buff *skb)
 			break;
 		case NFT_MSG_NEWCHAIN:
 			if (nft_trans_chain_update(trans)) {
-				if (nft_trans_chain_stats(trans))
-					free_percpu(nft_trans_chain_stats(trans));
+				free_percpu(nft_trans_chain_stats(trans));
 
 				nft_trans_destroy(trans);
 			} else {
-- 
2.1.3


^ permalink raw reply related

* Re: [PATCH] ipv6: delete protocol when cleanup
From: Cong Wang @ 2014-11-18 19:46 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: Duan Jiong, David Miller, netdev
In-Reply-To: <1416338055.14060.24.camel@edumazet-glaptop2.roam.corp.google.com>

On Tue, Nov 18, 2014 at 11:14 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Tue, 2014-11-18 at 10:56 -0800, Cong Wang wrote:
>> On Tue, Nov 18, 2014 at 2:35 AM, Duan Jiong <duanj.fnst@cn.fujitsu.com> wrote:
>> >
>> > pim6_protocol was added when initiation, but it not deleted.
>> >
>> > Signed-off-by: Duan Jiong <duanj.fnst@cn.fujitsu.com>
>
> ...
>
>> Looks like rtnl_unregister() is missing as well.
>
> Is it really worth bothering ?
>
> May I remind you we took the decision one year ago to not bother about
> removing ipv6 module.
>

It is still called in error path of init, so just for completeness.

^ permalink raw reply

* [PATCH 5/5] fold verify_iovec() into copy_msghdr_from_user()
From: Al Viro @ 2014-11-18 19:43 UTC (permalink / raw)
  To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>

... and do the same on the compat side of things.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/linux/socket.h |    1 -
 include/net/compat.h   |    5 ++-
 net/compat.c           |   52 ++++++++++++---------------
 net/core/iovec.c       |   38 --------------------
 net/socket.c           |   93 +++++++++++++++++++++++++++---------------------
 5 files changed, 77 insertions(+), 112 deletions(-)

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 51bd666..de52228 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -322,7 +322,6 @@ extern int csum_partial_copy_fromiovecend(unsigned char *kdata,
 extern unsigned long iov_pages(const struct iovec *iov, int offset,
 			       unsigned long nr_segs);
 
-extern int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode);
 extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
 extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
 
diff --git a/include/net/compat.h b/include/net/compat.h
index 3b603b1..42a9c84 100644
--- a/include/net/compat.h
+++ b/include/net/compat.h
@@ -40,9 +40,8 @@ int compat_sock_get_timestampns(struct sock *, struct timespec __user *);
 #define compat_mmsghdr	mmsghdr
 #endif /* defined(CONFIG_COMPAT) */
 
-int get_compat_msghdr(struct msghdr *, struct compat_msghdr __user *);
-int verify_compat_iovec(struct msghdr *, struct iovec *,
-			struct sockaddr_storage *, int);
+ssize_t get_compat_msghdr(struct msghdr *, struct compat_msghdr __user *,
+		      struct sockaddr __user **, struct iovec **);
 asmlinkage long compat_sys_sendmsg(int, struct compat_msghdr __user *,
 				   unsigned int);
 asmlinkage long compat_sys_sendmmsg(int, struct compat_mmsghdr __user *,
diff --git a/net/compat.c b/net/compat.c
index 7b4b6ad..062f157 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -31,14 +31,18 @@
 #include <asm/uaccess.h>
 #include <net/compat.h>
 
-int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
+ssize_t get_compat_msghdr(struct msghdr *kmsg,
+			  struct compat_msghdr __user *umsg,
+			  struct sockaddr __user **save_addr,
+			  struct iovec **iov)
 {
-	compat_uptr_t tmp1, tmp2, tmp3;
+	compat_uptr_t uaddr, uiov, tmp3;
+	ssize_t err;
 
 	if (!access_ok(VERIFY_READ, umsg, sizeof(*umsg)) ||
-	    __get_user(tmp1, &umsg->msg_name) ||
+	    __get_user(uaddr, &umsg->msg_name) ||
 	    __get_user(kmsg->msg_namelen, &umsg->msg_namelen) ||
-	    __get_user(tmp2, &umsg->msg_iov) ||
+	    __get_user(uiov, &umsg->msg_iov) ||
 	    __get_user(kmsg->msg_iovlen, &umsg->msg_iovlen) ||
 	    __get_user(tmp3, &umsg->msg_control) ||
 	    __get_user(kmsg->msg_controllen, &umsg->msg_controllen) ||
@@ -46,44 +50,32 @@ int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
 		return -EFAULT;
 	if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
 		kmsg->msg_namelen = sizeof(struct sockaddr_storage);
-	kmsg->msg_name = compat_ptr(tmp1);
-	kmsg->msg_iov = compat_ptr(tmp2);
 	kmsg->msg_control = compat_ptr(tmp3);
-	return 0;
-}
 
-/* I've named the args so it is easy to tell whose space the pointers are in. */
-int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *iov,
-		   struct sockaddr_storage *kern_address, int mode)
-{
-	struct compat_iovec __user *p;
-	struct iovec *res;
-	int err;
+	if (save_addr)
+		*save_addr = compat_ptr(uaddr);
 
-	if (kern_msg->msg_name && kern_msg->msg_namelen) {
-		if (mode == WRITE) {
-			int err = move_addr_to_kernel(kern_msg->msg_name,
-						      kern_msg->msg_namelen,
-						      kern_address);
+	if (uaddr && kmsg->msg_namelen) {
+		if (!save_addr) {
+			err = move_addr_to_kernel(compat_ptr(uaddr),
+						  kmsg->msg_namelen,
+						  kmsg->msg_name);
 			if (err < 0)
 				return err;
 		}
-		kern_msg->msg_name = kern_address;
 	} else {
-		kern_msg->msg_name = NULL;
-		kern_msg->msg_namelen = 0;
+		kmsg->msg_name = NULL;
+		kmsg->msg_namelen = 0;
 	}
 
-	if (kern_msg->msg_iovlen > UIO_MAXIOV)
+	if (kmsg->msg_iovlen > UIO_MAXIOV)
 		return -EMSGSIZE;
 
-	p = (struct compat_iovec __user *)kern_msg->msg_iov;
-	err = compat_rw_copy_check_uvector(mode, p, kern_msg->msg_iovlen,
-					   UIO_FASTIOV, iov, &res);
+	err = compat_rw_copy_check_uvector(save_addr ? READ : WRITE,
+					   compat_ptr(uiov), kmsg->msg_iovlen,
+					   UIO_FASTIOV, *iov, iov);
 	if (err >= 0)
-		kern_msg->msg_iov = res;
-	else if (res != iov)
-		kfree(res);
+		kmsg->msg_iov = *iov;
 	return err;
 }
 
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 4a35b21..fa3eed9 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -28,44 +28,6 @@
 #include <net/sock.h>
 
 /*
- *	Verify iovec. The caller must ensure that the iovec is big enough
- *	to hold the message iovec.
- *
- *	Save time not doing access_ok. copy_*_user will make this work
- *	in any case.
- */
-
-int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode)
-{
-	struct iovec *res;
-	int err;
-
-	if (m->msg_name && m->msg_namelen) {
-		if (mode == WRITE) {
-			void __user *namep = (void __user __force *)m->msg_name;
-			int err = move_addr_to_kernel(namep, m->msg_namelen,
-						  address);
-			if (err < 0)
-				return err;
-		}
-		m->msg_name = address;
-	} else {
-		m->msg_name = NULL;
-		m->msg_namelen = 0;
-	}
-	if (m->msg_iovlen > UIO_MAXIOV)
-		return -EMSGSIZE;
-
-	err = rw_copy_check_uvector(mode, (void __user __force *)m->msg_iov,
-				    m->msg_iovlen, UIO_FASTIOV, iov, &res);
-	if (err >= 0)
-		m->msg_iov = res;
-	else if (res != iov)
-		kfree(res);
-	return err;
-}
-
-/*
  *	And now for the all-in-one: copy and checksum from a user iovec
  *	directly to a datagram
  *	Calls to csum_partial but the last must be in 32 bit chunks
diff --git a/net/socket.c b/net/socket.c
index 59020f0..ee3ee39 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1988,16 +1988,26 @@ struct used_address {
 	unsigned int name_len;
 };
 
-static int copy_msghdr_from_user(struct msghdr *kmsg,
-				 struct user_msghdr __user *umsg)
+static ssize_t copy_msghdr_from_user(struct msghdr *kmsg,
+				     struct user_msghdr __user *umsg,
+				     struct sockaddr __user **save_addr,
+				     struct iovec **iov)
 {
-	/* We are relying on the (currently) identical layouts.  Once
-	 * the kernel-side changes, this place will need to be updated
-	 */
-	if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
+	struct sockaddr __user *uaddr;
+	struct iovec __user *uiov;
+	ssize_t err;
+
+	if (!access_ok(VERIFY_READ, umsg, sizeof(*umsg)) ||
+	    __get_user(uaddr, &umsg->msg_name) ||
+	    __get_user(kmsg->msg_namelen, &umsg->msg_namelen) ||
+	    __get_user(uiov, &umsg->msg_iov) ||
+	    __get_user(kmsg->msg_iovlen, &umsg->msg_iovlen) ||
+	    __get_user(kmsg->msg_control, &umsg->msg_control) ||
+	    __get_user(kmsg->msg_controllen, &umsg->msg_controllen) ||
+	    __get_user(kmsg->msg_flags, &umsg->msg_flags))
 		return -EFAULT;
 
-	if (kmsg->msg_name == NULL)
+	if (!uaddr)
 		kmsg->msg_namelen = 0;
 
 	if (kmsg->msg_namelen < 0)
@@ -2005,7 +2015,31 @@ static int copy_msghdr_from_user(struct msghdr *kmsg,
 
 	if (kmsg->msg_namelen > sizeof(struct sockaddr_storage))
 		kmsg->msg_namelen = sizeof(struct sockaddr_storage);
-	return 0;
+
+	if (save_addr)
+		*save_addr = uaddr;
+
+	if (uaddr && kmsg->msg_namelen) {
+		if (!save_addr) {
+			err = move_addr_to_kernel(uaddr, kmsg->msg_namelen,
+						  kmsg->msg_name);
+			if (err < 0)
+				return err;
+		}
+	} else {
+		kmsg->msg_name = NULL;
+		kmsg->msg_namelen = 0;
+	}
+
+	if (kmsg->msg_iovlen > UIO_MAXIOV)
+		return -EMSGSIZE;
+
+	err = rw_copy_check_uvector(save_addr ? READ : WRITE,
+				    uiov, kmsg->msg_iovlen,
+				    UIO_FASTIOV, *iov, iov);
+	if (err >= 0)
+		kmsg->msg_iov = *iov;
+	return err;
 }
 
 static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
@@ -2020,26 +2054,17 @@ static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
 	    __attribute__ ((aligned(sizeof(__kernel_size_t))));
 	/* 20 is size of ipv6_pktinfo */
 	unsigned char *ctl_buf = ctl;
-	int err, ctl_len, total_len;
+	int ctl_len, total_len;
+	ssize_t err;
 
-	err = -EFAULT;
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
-		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	msg_sys->msg_name = &address;
 
-	/* This will also move the address data into kernel space */
 	if (MSG_CMSG_COMPAT & flags)
-		err = verify_compat_iovec(msg_sys, iovstack, &address, WRITE);
+		err = get_compat_msghdr(msg_sys, msg_compat, NULL, &iov);
 	else
-		err = verify_iovec(msg_sys, iovstack, &address, WRITE);
+		err = copy_msghdr_from_user(msg_sys, msg, NULL, &iov);
 	if (err < 0)
 		goto out_freeiov;
-	iov = msg_sys->msg_iov;
 	total_len = err;
 
 	err = -ENOBUFS;
@@ -2215,36 +2240,24 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
 	struct iovec iovstack[UIO_FASTIOV];
 	struct iovec *iov = iovstack;
 	unsigned long cmsg_ptr;
-	int err, total_len, len;
+	int total_len, len;
+	ssize_t err;
 
 	/* kernel mode address */
 	struct sockaddr_storage addr;
 
 	/* user mode address pointers */
 	struct sockaddr __user *uaddr;
-	int __user *uaddr_len;
+	int __user *uaddr_len = COMPAT_NAMELEN(msg);
 
-	if (MSG_CMSG_COMPAT & flags) {
-		if (get_compat_msghdr(msg_sys, msg_compat))
-			return -EFAULT;
-	} else {
-		err = copy_msghdr_from_user(msg_sys, msg);
-		if (err)
-			return err;
-	}
+	msg_sys->msg_name = &addr;
 
-	/* Save the user-mode address (verify_iovec will change the
-	 * kernel msghdr to use the kernel address space)
-	 */
-	uaddr = (__force void __user *)msg_sys->msg_name;
-	uaddr_len = COMPAT_NAMELEN(msg);
 	if (MSG_CMSG_COMPAT & flags)
-		err = verify_compat_iovec(msg_sys, iovstack, &addr, READ);
+		err = get_compat_msghdr(msg_sys, msg_compat, &uaddr, &iov);
 	else
-		err = verify_iovec(msg_sys, iovstack, &addr, READ);
+		err = copy_msghdr_from_user(msg_sys, msg, &uaddr, &iov);
 	if (err < 0)
 		goto out_freeiov;
-	iov = msg_sys->msg_iov;
 	total_len = err;
 
 	cmsg_ptr = (unsigned long)msg_sys->msg_control;
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 4/5] bury skb_copy_to_page()
From: Al Viro @ 2014-11-18 19:43 UTC (permalink / raw)
  To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>

no callers since 3.0

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 include/net/sock.h |   23 -----------------------
 1 file changed, 23 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 94e0ead..d7d43e9 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1883,29 +1883,6 @@ static inline int skb_copy_to_page_nocache(struct sock *sk, char __user *from,
 	return 0;
 }
 
-static inline int skb_copy_to_page(struct sock *sk, char __user *from,
-				   struct sk_buff *skb, struct page *page,
-				   int off, int copy)
-{
-	if (skb->ip_summed == CHECKSUM_NONE) {
-		int err = 0;
-		__wsum csum = csum_and_copy_from_user(from,
-						     page_address(page) + off,
-							    copy, 0, &err);
-		if (err)
-			return err;
-		skb->csum = csum_block_add(skb->csum, csum, skb->len);
-	} else if (__copy_from_user(page_address(page) + off, from, copy))
-		return -EFAULT;
-
-	skb->len	     += copy;
-	skb->data_len	     += copy;
-	skb->truesize	     += copy;
-	sk->sk_wmem_queued   += copy;
-	sk_mem_charge(sk, copy);
-	return 0;
-}
-
 /**
  * sk_wmem_alloc_get - returns write allocations
  * @sk: socket
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 3/5] remove a bunch of now-pointless access_ok() in net
From: Al Viro @ 2014-11-18 19:42 UTC (permalink / raw)
  To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>

The following set of functions
	skb_add_data_nocache
	skb_copy_to_page_nocache
	skb_do_copy_data_nocache
	skb_copy_to_page
	skb_add_data
	csum_and_copy_from_user
	skb_copy_and_csum_datagram
	csum_and_copy_to_user
	memcpy_fromiovec
	memcpy_toiovec
	memcpy_toiovecend
	memcpy_fromiovecend
are never given a userland range that would not satisfy access_ok().

Proof:

1) skb_add_data_nocache() and skb_copy_to_page_nocache() are called only
by tcp_sendmsg() and given a range covered by ->msg_iov[].

2) skb_do_copy_data_nocache() is called only by skb_add_data_nocache() and
skb_copy_to_page_nocache() and range comes from their arguments.

3) skb_copy_to_page() is never called at all (dead code since 3.0; killed in
the next commit).

4) skb_add_data() is called by rxrpc_send_data() and tcp_send_syn_data(),
both passing it a range covered by ->msg_iov[].

5) all callers of csum_partial_copy_fromiovecend() are giving it iovecs from
->msg_iov[].  Proof: it is called by ip_generic_getfrag() and ping_getfrag(),
both are called only as callbacks by ip_append_data(), ip6_append_data() and
ip_make_skb() and argument passed to those callbacks in all such invocations
is ->msg_iov.

6) csum_and_copy_from_user() is called by skb_add_data(), skb_copy_to_page(),
skb_do_copy_data_nocache() and csum_partial_copy_fromiovecend().  In all cases
the range is covered by ->msg_iov[]

7) skb_copy_and_csum_datagram_iovec() is always getting an iovec from ->msg_iov.
Proof: it is called by tcp_copy_to_iovec(), which gives it tp->ucopy.iov,
and by several recvmsg instances (udp, raw, raw6) which give it ->msg_iov.
But tp->ucopy.iov is initialized only by ->msg_iov.

8) skb_copy_and_csum_datagram() is called only by itself (for fragments)
and by skb_copy_and_csum_datagram_iovec().  The range is covered by the range
passed to caller (in the first case) or by an iovec passed to
skb_copy_and_csum_datagram_iovec().

9) csum_and_copy_to_user() is called only by skb_copy_and_csum_datagram().
Range is covered by the range given to caller...

10) skb_copy_datagram_iovec() is always getting an iovec that would pass
access_ok() on all elements.  Proof: cases when ->msg_iov or ->ucopy.iov are
passed are trivial.  Other than those, we have
	* ppp_read() (single-element iovec, range passed to ->read() has been
validated by caller)
	* skb_copy_and_csum_datagram_iovec() (see (7)), itself (covered by
the ranges in array given to its caller)
	* rds_tcp_inc_copy_to_user(), which is called only as
->inc_copy_to_user(), which is always given ->msg_iov.

11) aside of the callers of memcpy_toiovec() that immediately pass it ->msg_iov,
there are 3 call sites: one in __qp_memcpy_from_queue() (when called from
qp_memcpy_from_queue_iov()) and two in skb_copy_datagram_iovec().  The latter
is OK due to (10), the former has the call chain coming through
vmci_qpair_dequev() and vmci_transport_stream_dequeue(), which is called as
->stream_dequeue(), which is always given ->msg_iov.
Types in vmw_vmci blow, film at 11...

12) memcpy_toiovecend() is always given a subset of something we'd just given
to ->recvmsg() in ->msg_iov.

13) most of the memcpy_fromiovec() callers are explicitly passing it ->msg_iov.
There are few exceptions:
	* l2cap_skbuff_fromiovec().  Called only as bluetooth
->memcpy_fromiovec(), which always gets ->msg_iov as argument.
	* __qp_memcpy_to_queue(), from qp_memcpy_to_queue_iov(), from
vmci_qpair_enquev(), from vmci_transport_stream_enqueue(), which is always
called as ->stream_enqueue(), which always gets ->msg_iov.  Don't ask me what
I think of vmware...
	* ipxrtr_route_packet(), which is always given ->msg_iov by its caller.
	* vmci_transport_dgram_enqueue(), which is always called as
->dgram_enqueue(), which always gets ->msg_iov.
	* vhost get_indirect(), which is passing it iovec filled by
translate_desc().  Ranges are subsets of those that had been validated by
vq_memory_access_ok() back when we did vhost_set_memory().

14) zerocopy_sg_from_iovec() always gets a validated iovec.
Proof: callers are {tun,macvtap}_get_user(), which are called either from
->aio_write() (and given iovec validated by caller of ->aio_write()), or
from ->sendmsg() (and given ->msg_iov).

15) skb_copy_datagram_from_iovec() always gets an validated iovec.
Proof: for callers in macvtap and tun, same as in (14).  Ones in net/unix
and net/packet are given ->msg_iov.  Other than those, there's one
in zerocopy_sg_from_iovec() (see (14)) and one in skb_copy_datagram_from_iovec()
itself (fragments handling) - that one gets the iovec its caller was given.

16) callers of memcpy_fromiovecend() are
	* {macvtap,tun}_get_user().  Same as (14).
	* skb_copy_datagram_from_iovec().  See (15).
	* raw_send_hdrinc(), raw6_send_hdrinv().  Both get ->msg_iov as argument
from their callers and pass it to memcpy_fromiovecend().
	* sctp_user_addto_chunk().  Ditto.
	* tipc_msg_build().  Again, iovec argument is always ->msg_iov.
	* ip_generic_getfrag() and udplite_getfrag().  Same as (5).
	* vhost_scsi_handle_vq() - that one gets vq->iov as iovec, and
vq->iov is filled ultimately by translate_desc().  Validated by
vq_memory_access_ok() back when we did vhost_set_memory().

And anything that might end up in ->msg_iov[] has to pass access_ok().  It's
trivial
for kernel_{send,recv}msg() users (there we are under set_fs(KERNEL_DS)), it's
verified by rw_copy_check_uvector() in sendmsg()/recvmsg()/sendmmsg()/recvmmsg()
and in the only place where we call ->sendmsg()/->recvmsg() not via net/socket.c
helpers (drivers/vhost/net.c) they are getting vq->iov.  As mentioned above,
this one is guaranteed to pass the checks since it's filled by translate_desc(),
and ranges it fills are subsets of ranges that had been validated when we
did vhost_set_memory().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/alpha/lib/csum_partial_copy.c      |    5 ----
 arch/frv/lib/checksum.c                 |    2 +-
 arch/m32r/lib/csum_partial_copy.c       |    2 +-
 arch/mips/include/asm/checksum.h        |   30 +++++++--------------
 arch/mn10300/lib/checksum.c             |    4 +--
 arch/parisc/include/asm/checksum.h      |    2 +-
 arch/parisc/lib/checksum.c              |    2 +-
 arch/powerpc/lib/checksum_wrappers_64.c |    6 ++---
 arch/s390/include/asm/checksum.h        |    2 +-
 arch/score/include/asm/checksum.h       |    2 +-
 arch/score/lib/checksum_copy.c          |    2 +-
 arch/sh/include/asm/checksum_32.h       |    8 +-----
 arch/sparc/include/asm/checksum_32.h    |   43 ++++++++++++++-----------------
 arch/x86/include/asm/checksum_32.h      |   17 ++++--------
 arch/x86/lib/csum-wrappers_64.c         |    3 ---
 arch/x86/um/asm/checksum.h              |    2 +-
 arch/x86/um/asm/checksum_32.h           |   12 ++++-----
 arch/xtensa/include/asm/checksum.h      |    8 +-----
 include/linux/skbuff.h                  |    2 +-
 include/net/checksum.h                  |   14 +++-------
 include/net/sock.h                      |    7 +++--
 lib/iovec.c                             |    8 +++---
 net/core/iovec.c                        |    6 ++---
 23 files changed, 67 insertions(+), 122 deletions(-)

diff --git a/arch/alpha/lib/csum_partial_copy.c b/arch/alpha/lib/csum_partial_copy.c
index 5675dca..1ee80e8 100644
--- a/arch/alpha/lib/csum_partial_copy.c
+++ b/arch/alpha/lib/csum_partial_copy.c
@@ -338,11 +338,6 @@ csum_partial_copy_from_user(const void __user *src, void *dst, int len,
 	unsigned long doff = 7 & (unsigned long) dst;
 
 	if (len) {
-		if (!access_ok(VERIFY_READ, src, len)) {
-			if (errp) *errp = -EFAULT;
-			memset(dst, 0, len);
-			return sum;
-		}
 		if (!doff) {
 			if (!soff)
 				checksum = csum_partial_cfu_aligned(
diff --git a/arch/frv/lib/checksum.c b/arch/frv/lib/checksum.c
index 44e16d5..f3fd6cd 100644
--- a/arch/frv/lib/checksum.c
+++ b/arch/frv/lib/checksum.c
@@ -140,7 +140,7 @@ csum_partial_copy_from_user(const void __user *src, void *dst,
 	if (csum_err)
 		*csum_err = 0;
 
-	rem = copy_from_user(dst, src, len);
+	rem = __copy_from_user(dst, src, len);
 	if (rem != 0) {
 		if (csum_err)
 			*csum_err = -EFAULT;
diff --git a/arch/m32r/lib/csum_partial_copy.c b/arch/m32r/lib/csum_partial_copy.c
index 5596f3d..f307c53 100644
--- a/arch/m32r/lib/csum_partial_copy.c
+++ b/arch/m32r/lib/csum_partial_copy.c
@@ -47,7 +47,7 @@ csum_partial_copy_from_user (const void __user *src, void *dst,
 {
 	int missing;
 
-	missing = copy_from_user(dst, src, len);
+	missing = __copy_from_user(dst, src, len);
 	if (missing) {
 		memset(dst + len - missing, 0, missing);
 		*err_ptr = -EFAULT;
diff --git a/arch/mips/include/asm/checksum.h b/arch/mips/include/asm/checksum.h
index 3418c51..80f48c4 100644
--- a/arch/mips/include/asm/checksum.h
+++ b/arch/mips/include/asm/checksum.h
@@ -59,13 +59,7 @@ static inline
 __wsum csum_and_copy_from_user(const void __user *src, void *dst,
 			       int len, __wsum sum, int *err_ptr)
 {
-	if (access_ok(VERIFY_READ, src, len))
-		return csum_partial_copy_from_user(src, dst, len, sum,
-						   err_ptr);
-	if (len)
-		*err_ptr = -EFAULT;
-
-	return sum;
+	return csum_partial_copy_from_user(src, dst, len, sum, err_ptr);
 }
 
 /*
@@ -77,20 +71,14 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
 			     __wsum sum, int *err_ptr)
 {
 	might_fault();
-	if (access_ok(VERIFY_WRITE, dst, len)) {
-		if (segment_eq(get_fs(), get_ds()))
-			return __csum_partial_copy_kernel(src,
-							  (__force void *)dst,
-							  len, sum, err_ptr);
-		else
-			return __csum_partial_copy_to_user(src,
-							   (__force void *)dst,
-							   len, sum, err_ptr);
-	}
-	if (len)
-		*err_ptr = -EFAULT;
-
-	return (__force __wsum)-1; /* invalid checksum */
+	if (segment_eq(get_fs(), get_ds()))
+		return __csum_partial_copy_kernel(src,
+						  (__force void *)dst,
+						  len, sum, err_ptr);
+	else
+		return __csum_partial_copy_to_user(src,
+						   (__force void *)dst,
+						   len, sum, err_ptr);
 }
 
 /*
diff --git a/arch/mn10300/lib/checksum.c b/arch/mn10300/lib/checksum.c
index b6580f5..d080d5e 100644
--- a/arch/mn10300/lib/checksum.c
+++ b/arch/mn10300/lib/checksum.c
@@ -73,7 +73,7 @@ __wsum csum_partial_copy_from_user(const void *src, void *dst,
 {
 	int missing;
 
-	missing = copy_from_user(dst, src, len);
+	missing = __copy_from_user(dst, src, len);
 	if (missing) {
 		memset(dst + len - missing, 0, missing);
 		*err_ptr = -EFAULT;
@@ -89,7 +89,7 @@ __wsum csum_and_copy_to_user(const void *src, void *dst,
 {
 	int missing;
 
-	missing = copy_to_user(dst, src, len);
+	missing = __copy_to_user(dst, src, len);
 	if (missing) {
 		memset(dst + len - missing, 0, missing);
 		*err_ptr = -EFAULT;
diff --git a/arch/parisc/include/asm/checksum.h b/arch/parisc/include/asm/checksum.h
index c84b2fc..6ca9a6f 100644
--- a/arch/parisc/include/asm/checksum.h
+++ b/arch/parisc/include/asm/checksum.h
@@ -198,7 +198,7 @@ static __inline__ __wsum csum_and_copy_to_user(const void *src,
 	/* code stolen from include/asm-mips64 */
 	sum = csum_partial(src, len, sum);
 	 
-	if (copy_to_user(dst, src, len)) {
+	if (__copy_to_user(dst, src, len)) {
 		*err_ptr = -EFAULT;
 		return (__force __wsum)-1;
 	}
diff --git a/arch/parisc/lib/checksum.c b/arch/parisc/lib/checksum.c
index ae66d31..e52fdba 100644
--- a/arch/parisc/lib/checksum.c
+++ b/arch/parisc/lib/checksum.c
@@ -138,7 +138,7 @@ __wsum csum_partial_copy_from_user(const void __user *src,
 {
 	int missing;
 
-	missing = copy_from_user(dst, src, len);
+	missing = __copy_from_user(dst, src, len);
 	if (missing) {
 		memset(dst + len - missing, 0, missing);
 		*err_ptr = -EFAULT;
diff --git a/arch/powerpc/lib/checksum_wrappers_64.c b/arch/powerpc/lib/checksum_wrappers_64.c
index 08e3a33..4c783c8 100644
--- a/arch/powerpc/lib/checksum_wrappers_64.c
+++ b/arch/powerpc/lib/checksum_wrappers_64.c
@@ -37,7 +37,7 @@ __wsum csum_and_copy_from_user(const void __user *src, void *dst,
 		goto out;
 	}
 
-	if (unlikely((len < 0) || !access_ok(VERIFY_READ, src, len))) {
+	if (unlikely(len < 0)) {
 		*err_ptr = -EFAULT;
 		csum = (__force unsigned int)sum;
 		goto out;
@@ -78,7 +78,7 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
 		goto out;
 	}
 
-	if (unlikely((len < 0) || !access_ok(VERIFY_WRITE, dst, len))) {
+	if (unlikely(len < 0)) {
 		*err_ptr = -EFAULT;
 		csum = -1; /* invalid checksum */
 		goto out;
@@ -90,7 +90,7 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
 	if (unlikely(*err_ptr)) {
 		csum = csum_partial(src, len, sum);
 
-		if (copy_to_user(dst, src, len)) {
+		if (__copy_to_user(dst, src, len)) {
 			*err_ptr = -EFAULT;
 			csum = -1; /* invalid checksum */
 		}
diff --git a/arch/s390/include/asm/checksum.h b/arch/s390/include/asm/checksum.h
index 7403648..fdc9532 100644
--- a/arch/s390/include/asm/checksum.h
+++ b/arch/s390/include/asm/checksum.h
@@ -51,7 +51,7 @@ csum_partial_copy_from_user(const void __user *src, void *dst,
                                           int len, __wsum sum,
                                           int *err_ptr)
 {
-	if (unlikely(copy_from_user(dst, src, len)))
+	if (unlikely(__copy_from_user(dst, src, len)))
 		*err_ptr = -EFAULT;
 	return csum_partial(dst, len, sum);
 }
diff --git a/arch/score/include/asm/checksum.h b/arch/score/include/asm/checksum.h
index 961bd64..d783634 100644
--- a/arch/score/include/asm/checksum.h
+++ b/arch/score/include/asm/checksum.h
@@ -36,7 +36,7 @@ __wsum csum_and_copy_to_user(const void *src, void __user *dst, int len,
 			__wsum sum, int *err_ptr)
 {
 	sum = csum_partial(src, len, sum);
-	if (copy_to_user(dst, src, len)) {
+	if (__copy_to_user(dst, src, len)) {
 		*err_ptr = -EFAULT;
 		return (__force __wsum) -1; /* invalid checksum */
 	}
diff --git a/arch/score/lib/checksum_copy.c b/arch/score/lib/checksum_copy.c
index 9b770b3..7d6b9b4 100644
--- a/arch/score/lib/checksum_copy.c
+++ b/arch/score/lib/checksum_copy.c
@@ -42,7 +42,7 @@ unsigned int csum_partial_copy_from_user(const char *src, char *dst,
 {
 	int missing;
 
-	missing = copy_from_user(dst, src, len);
+	missing = __copy_from_user(dst, src, len);
 	if (missing) {
 		memset(dst + len - missing, 0, missing);
 		*err_ptr = -EFAULT;
diff --git a/arch/sh/include/asm/checksum_32.h b/arch/sh/include/asm/checksum_32.h
index 14b7ac2..9749277 100644
--- a/arch/sh/include/asm/checksum_32.h
+++ b/arch/sh/include/asm/checksum_32.h
@@ -203,13 +203,7 @@ static inline __wsum csum_and_copy_to_user(const void *src,
 					   int len, __wsum sum,
 					   int *err_ptr)
 {
-	if (access_ok(VERIFY_WRITE, dst, len))
-		return csum_partial_copy_generic((__force const void *)src,
+	return csum_partial_copy_generic((__force const void *)src,
 						dst, len, sum, NULL, err_ptr);
-
-	if (len)
-		*err_ptr = -EFAULT;
-
-	return (__force __wsum)-1; /* invalid checksum */
 }
 #endif /* __ASM_SH_CHECKSUM_H */
diff --git a/arch/sparc/include/asm/checksum_32.h b/arch/sparc/include/asm/checksum_32.h
index 426b238..be2e295 100644
--- a/arch/sparc/include/asm/checksum_32.h
+++ b/arch/sparc/include/asm/checksum_32.h
@@ -86,30 +86,25 @@ static inline __wsum
 csum_partial_copy_to_user(const void *src, void __user *dst, int len,
 			  __wsum sum, int *err)
 {
-	if (!access_ok (VERIFY_WRITE, dst, len)) {
-		*err = -EFAULT;
-		return sum;
-	} else {
-		register unsigned long ret asm("o0") = (unsigned long)src;
-		register char __user *d asm("o1") = dst;
-		register int l asm("g1") = len;
-		register __wsum s asm("g7") = sum;
-
-		__asm__ __volatile__ (
-		".section __ex_table,#alloc\n\t"
-		".align 4\n\t"
-		".word 1f,1\n\t"
-		".previous\n"
-		"1:\n\t"
-		"call __csum_partial_copy_sparc_generic\n\t"
-		" st %8, [%%sp + 64]\n"
-		: "=&r" (ret), "=&r" (d), "=&r" (l), "=&r" (s)
-		: "0" (ret), "1" (d), "2" (l), "3" (s), "r" (err)
-		: "o2", "o3", "o4", "o5", "o7",
-		  "g2", "g3", "g4", "g5",
-		  "cc", "memory");
-		return (__force __wsum)ret;
-	}
+	register unsigned long ret asm("o0") = (unsigned long)src;
+	register char __user *d asm("o1") = dst;
+	register int l asm("g1") = len;
+	register __wsum s asm("g7") = sum;
+
+	__asm__ __volatile__ (
+	".section __ex_table,#alloc\n\t"
+	".align 4\n\t"
+	".word 1f,1\n\t"
+	".previous\n"
+	"1:\n\t"
+	"call __csum_partial_copy_sparc_generic\n\t"
+	" st %8, [%%sp + 64]\n"
+	: "=&r" (ret), "=&r" (d), "=&r" (l), "=&r" (s)
+	: "0" (ret), "1" (d), "2" (l), "3" (s), "r" (err)
+	: "o2", "o3", "o4", "o5", "o7",
+	  "g2", "g3", "g4", "g5",
+	  "cc", "memory");
+	return (__force __wsum)ret;
 }
 
 #define HAVE_CSUM_COPY_USER
diff --git a/arch/x86/include/asm/checksum_32.h b/arch/x86/include/asm/checksum_32.h
index f50de69..4d96e08 100644
--- a/arch/x86/include/asm/checksum_32.h
+++ b/arch/x86/include/asm/checksum_32.h
@@ -185,18 +185,11 @@ static inline __wsum csum_and_copy_to_user(const void *src,
 	__wsum ret;
 
 	might_sleep();
-	if (access_ok(VERIFY_WRITE, dst, len)) {
-		stac();
-		ret = csum_partial_copy_generic(src, (__force void *)dst,
-						len, sum, NULL, err_ptr);
-		clac();
-		return ret;
-	}
-
-	if (len)
-		*err_ptr = -EFAULT;
-
-	return (__force __wsum)-1; /* invalid checksum */
+	stac();
+	ret = csum_partial_copy_generic(src, (__force void *)dst,
+					len, sum, NULL, err_ptr);
+	clac();
+	return ret;
 }
 
 #endif /* _ASM_X86_CHECKSUM_32_H */
diff --git a/arch/x86/lib/csum-wrappers_64.c b/arch/x86/lib/csum-wrappers_64.c
index 7609e0e..b6b5626 100644
--- a/arch/x86/lib/csum-wrappers_64.c
+++ b/arch/x86/lib/csum-wrappers_64.c
@@ -26,9 +26,6 @@ csum_partial_copy_from_user(const void __user *src, void *dst,
 	might_sleep();
 	*errp = 0;
 
-	if (!likely(access_ok(VERIFY_READ, src, len)))
-		goto out_err;
-
 	/*
 	 * Why 6, not 7? To handle odd addresses aligned we
 	 * would need to do considerable complications to fix the
diff --git a/arch/x86/um/asm/checksum.h b/arch/x86/um/asm/checksum.h
index 4b181b7..3a5166c 100644
--- a/arch/x86/um/asm/checksum.h
+++ b/arch/x86/um/asm/checksum.h
@@ -46,7 +46,7 @@ static __inline__
 __wsum csum_partial_copy_from_user(const void __user *src, void *dst,
 					 int len, __wsum sum, int *err_ptr)
 {
-	if (copy_from_user(dst, src, len)) {
+	if (__copy_from_user(dst, src, len)) {
 		*err_ptr = -EFAULT;
 		return (__force __wsum)-1;
 	}
diff --git a/arch/x86/um/asm/checksum_32.h b/arch/x86/um/asm/checksum_32.h
index ab77b6f..e047748 100644
--- a/arch/x86/um/asm/checksum_32.h
+++ b/arch/x86/um/asm/checksum_32.h
@@ -43,14 +43,12 @@ static __inline__ __wsum csum_and_copy_to_user(const void *src,
 						     void __user *dst,
 						     int len, __wsum sum, int *err_ptr)
 {
-	if (access_ok(VERIFY_WRITE, dst, len)) {
-		if (copy_to_user(dst, src, len)) {
-			*err_ptr = -EFAULT;
-			return (__force __wsum)-1;
-		}
-
-		return csum_partial(src, len, sum);
+	if (__copy_to_user(dst, src, len)) {
+		*err_ptr = -EFAULT;
+		return (__force __wsum)-1;
 	}
+	return csum_partial(src, len, sum);
+}
 
 	if (len)
 		*err_ptr = -EFAULT;
diff --git a/arch/xtensa/include/asm/checksum.h b/arch/xtensa/include/asm/checksum.h
index 0593de6..e2fd018 100644
--- a/arch/xtensa/include/asm/checksum.h
+++ b/arch/xtensa/include/asm/checksum.h
@@ -245,12 +245,6 @@ static __inline__ __wsum csum_and_copy_to_user(const void *src,
 					       void __user *dst, int len,
 					       __wsum sum, int *err_ptr)
 {
-	if (access_ok(VERIFY_WRITE, dst, len))
-		return csum_partial_copy_generic(src,dst,len,sum,NULL,err_ptr);
-
-	if (len)
-		*err_ptr = -EFAULT;
-
-	return (__force __wsum)-1; /* invalid checksum */
+	return csum_partial_copy_generic(src,dst,len,sum,NULL,err_ptr);
 }
 #endif
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index 73c370e..df99075 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -2483,7 +2483,7 @@ static inline int skb_add_data(struct sk_buff *skb,
 			skb->csum = csum_block_add(skb->csum, csum, off);
 			return 0;
 		}
-	} else if (!copy_from_user(skb_put(skb, copy), from, copy))
+	} else if (!__copy_from_user(skb_put(skb, copy), from, copy))
 		return 0;
 
 	__skb_trim(skb, off);
diff --git a/include/net/checksum.h b/include/net/checksum.h
index 6465bae..3ed85c8 100644
--- a/include/net/checksum.h
+++ b/include/net/checksum.h
@@ -30,13 +30,7 @@ static inline
 __wsum csum_and_copy_from_user (const void __user *src, void *dst,
 				      int len, __wsum sum, int *err_ptr)
 {
-	if (access_ok(VERIFY_READ, src, len))
-		return csum_partial_copy_from_user(src, dst, len, sum, err_ptr);
-
-	if (len)
-		*err_ptr = -EFAULT;
-
-	return sum;
+	return csum_partial_copy_from_user(src, dst, len, sum, err_ptr);
 }
 #endif
 
@@ -46,10 +40,8 @@ static __inline__ __wsum csum_and_copy_to_user
 {
 	sum = csum_partial(src, len, sum);
 
-	if (access_ok(VERIFY_WRITE, dst, len)) {
-		if (copy_to_user(dst, src, len) == 0)
-			return sum;
-	}
+	if (__copy_to_user(dst, src, len) == 0)
+		return sum;
 	if (len)
 		*err_ptr = -EFAULT;
 
diff --git a/include/net/sock.h b/include/net/sock.h
index 83a669f..94e0ead 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -1842,10 +1842,9 @@ static inline int skb_do_copy_data_nocache(struct sock *sk, struct sk_buff *skb,
 			return err;
 		skb->csum = csum_block_add(skb->csum, csum, offset);
 	} else if (sk->sk_route_caps & NETIF_F_NOCACHE_COPY) {
-		if (!access_ok(VERIFY_READ, from, copy) ||
-		    __copy_from_user_nocache(to, from, copy))
+		if (__copy_from_user_nocache(to, from, copy))
 			return -EFAULT;
-	} else if (copy_from_user(to, from, copy))
+	} else if (__copy_from_user(to, from, copy))
 		return -EFAULT;
 
 	return 0;
@@ -1896,7 +1895,7 @@ static inline int skb_copy_to_page(struct sock *sk, char __user *from,
 		if (err)
 			return err;
 		skb->csum = csum_block_add(skb->csum, csum, skb->len);
-	} else if (copy_from_user(page_address(page) + off, from, copy))
+	} else if (__copy_from_user(page_address(page) + off, from, copy))
 		return -EFAULT;
 
 	skb->len	     += copy;
diff --git a/lib/iovec.c b/lib/iovec.c
index df3abd1..021d53f 100644
--- a/lib/iovec.c
+++ b/lib/iovec.c
@@ -13,7 +13,7 @@ int memcpy_fromiovec(unsigned char *kdata, struct iovec *iov, int len)
 	while (len > 0) {
 		if (iov->iov_len) {
 			int copy = min_t(unsigned int, len, iov->iov_len);
-			if (copy_from_user(kdata, iov->iov_base, copy))
+			if (__copy_from_user(kdata, iov->iov_base, copy))
 				return -EFAULT;
 			len -= copy;
 			kdata += copy;
@@ -38,7 +38,7 @@ int memcpy_toiovec(struct iovec *iov, unsigned char *kdata, int len)
 	while (len > 0) {
 		if (iov->iov_len) {
 			int copy = min_t(unsigned int, iov->iov_len, len);
-			if (copy_to_user(iov->iov_base, kdata, copy))
+			if (__copy_to_user(iov->iov_base, kdata, copy))
 				return -EFAULT;
 			kdata += copy;
 			len -= copy;
@@ -67,7 +67,7 @@ int memcpy_toiovecend(const struct iovec *iov, unsigned char *kdata,
 			continue;
 		}
 		copy = min_t(unsigned int, iov->iov_len - offset, len);
-		if (copy_to_user(iov->iov_base + offset, kdata, copy))
+		if (__copy_to_user(iov->iov_base + offset, kdata, copy))
 			return -EFAULT;
 		offset = 0;
 		kdata += copy;
@@ -100,7 +100,7 @@ int memcpy_fromiovecend(unsigned char *kdata, const struct iovec *iov,
 		int copy = min_t(unsigned int, len, iov->iov_len - offset);
 
 		offset = 0;
-		if (copy_from_user(kdata, base, copy))
+		if (__copy_from_user(kdata, base, copy))
 			return -EFAULT;
 		len -= copy;
 		kdata += copy;
diff --git a/net/core/iovec.c b/net/core/iovec.c
index 86beeea..4a35b21 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -97,7 +97,7 @@ int csum_partial_copy_fromiovecend(unsigned char *kdata, struct iovec *iov,
 
 			/* iov component is too short ... */
 			if (par_len > copy) {
-				if (copy_from_user(kdata, base, copy))
+				if (__copy_from_user(kdata, base, copy))
 					goto out_fault;
 				kdata += copy;
 				base += copy;
@@ -110,7 +110,7 @@ int csum_partial_copy_fromiovecend(unsigned char *kdata, struct iovec *iov,
 							 partial_cnt, csum);
 				goto out;
 			}
-			if (copy_from_user(kdata, base, par_len))
+			if (__copy_from_user(kdata, base, par_len))
 				goto out_fault;
 			csum = csum_partial(kdata - partial_cnt, 4, csum);
 			kdata += par_len;
@@ -124,7 +124,7 @@ int csum_partial_copy_fromiovecend(unsigned char *kdata, struct iovec *iov,
 			partial_cnt = copy % 4;
 			if (partial_cnt) {
 				copy -= partial_cnt;
-				if (copy_from_user(kdata + copy, base + copy,
+				if (__copy_from_user(kdata + copy, base + copy,
 						partial_cnt))
 					goto out_fault;
 			}
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 2/5] {compat_,}verify_iovec(): switch to generic copying of iovecs
From: Al Viro @ 2014-11-18 19:42 UTC (permalink / raw)
  To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>

use {compat_,}rw_copy_check_uvector().  As the result, we are
guaranteed that all iovecs seen in ->msg_iov by ->sendmsg()
and ->recvmsg() will pass access_ok().  The next commit removes
now redundant checks in callees...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 net/compat.c     |   51 +++++++++++++++------------------------------------
 net/core/iovec.c |   37 ++++++++++++++-----------------------
 net/socket.c     |   38 ++++++++------------------------------
 3 files changed, 37 insertions(+), 89 deletions(-)

diff --git a/net/compat.c b/net/compat.c
index 562e920..7b4b6ad 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -31,33 +31,6 @@
 #include <asm/uaccess.h>
 #include <net/compat.h>
 
-static inline int iov_from_user_compat_to_kern(struct iovec *kiov,
-					  struct compat_iovec __user *uiov32,
-					  int niov)
-{
-	int tot_len = 0;
-
-	while (niov > 0) {
-		compat_uptr_t buf;
-		compat_size_t len;
-
-		if (get_user(len, &uiov32->iov_len) ||
-		    get_user(buf, &uiov32->iov_base))
-			return -EFAULT;
-
-		if (len > INT_MAX - tot_len)
-			len = INT_MAX - tot_len;
-
-		tot_len += len;
-		kiov->iov_base = compat_ptr(buf);
-		kiov->iov_len = (__kernel_size_t) len;
-		uiov32++;
-		kiov++;
-		niov--;
-	}
-	return tot_len;
-}
-
 int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
 {
 	compat_uptr_t tmp1, tmp2, tmp3;
@@ -80,13 +53,15 @@ int get_compat_msghdr(struct msghdr *kmsg, struct compat_msghdr __user *umsg)
 }
 
 /* I've named the args so it is easy to tell whose space the pointers are in. */
-int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
+int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *iov,
 		   struct sockaddr_storage *kern_address, int mode)
 {
-	int tot_len;
+	struct compat_iovec __user *p;
+	struct iovec *res;
+	int err;
 
 	if (kern_msg->msg_name && kern_msg->msg_namelen) {
-		if (mode == VERIFY_READ) {
+		if (mode == WRITE) {
 			int err = move_addr_to_kernel(kern_msg->msg_name,
 						      kern_msg->msg_namelen,
 						      kern_address);
@@ -99,13 +74,17 @@ int verify_compat_iovec(struct msghdr *kern_msg, struct iovec *kern_iov,
 		kern_msg->msg_namelen = 0;
 	}
 
-	tot_len = iov_from_user_compat_to_kern(kern_iov,
-					  (struct compat_iovec __user *)kern_msg->msg_iov,
-					  kern_msg->msg_iovlen);
-	if (tot_len >= 0)
-		kern_msg->msg_iov = kern_iov;
+	if (kern_msg->msg_iovlen > UIO_MAXIOV)
+		return -EMSGSIZE;
 
-	return tot_len;
+	p = (struct compat_iovec __user *)kern_msg->msg_iov;
+	err = compat_rw_copy_check_uvector(mode, p, kern_msg->msg_iovlen,
+					   UIO_FASTIOV, iov, &res);
+	if (err >= 0)
+		kern_msg->msg_iov = res;
+	else if (res != iov)
+		kfree(res);
+	return err;
 }
 
 /* Bleech... */
diff --git a/net/core/iovec.c b/net/core/iovec.c
index e1ec45a..86beeea 100644
--- a/net/core/iovec.c
+++ b/net/core/iovec.c
@@ -37,13 +37,13 @@
 
 int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *address, int mode)
 {
-	int size, ct, err;
+	struct iovec *res;
+	int err;
 
 	if (m->msg_name && m->msg_namelen) {
-		if (mode == VERIFY_READ) {
-			void __user *namep;
-			namep = (void __user __force *) m->msg_name;
-			err = move_addr_to_kernel(namep, m->msg_namelen,
+		if (mode == WRITE) {
+			void __user *namep = (void __user __force *)m->msg_name;
+			int err = move_addr_to_kernel(namep, m->msg_namelen,
 						  address);
 			if (err < 0)
 				return err;
@@ -53,24 +53,15 @@ int verify_iovec(struct msghdr *m, struct iovec *iov, struct sockaddr_storage *a
 		m->msg_name = NULL;
 		m->msg_namelen = 0;
 	}
-
-	size = m->msg_iovlen * sizeof(struct iovec);
-	if (copy_from_user(iov, (void __user __force *) m->msg_iov, size))
-		return -EFAULT;
-
-	m->msg_iov = iov;
-	err = 0;
-
-	for (ct = 0; ct < m->msg_iovlen; ct++) {
-		size_t len = iov[ct].iov_len;
-
-		if (len > INT_MAX - err) {
-			len = INT_MAX - err;
-			iov[ct].iov_len = len;
-		}
-		err += len;
-	}
-
+	if (m->msg_iovlen > UIO_MAXIOV)
+		return -EMSGSIZE;
+
+	err = rw_copy_check_uvector(mode, (void __user __force *)m->msg_iov,
+				    m->msg_iovlen, UIO_FASTIOV, iov, &res);
+	if (err >= 0)
+		m->msg_iov = res;
+	else if (res != iov)
+		kfree(res);
 	return err;
 }
 
diff --git a/net/socket.c b/net/socket.c
index 0ae8147..59020f0 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2032,24 +2032,14 @@ static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
 			return err;
 	}
 
-	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
-		err = -EMSGSIZE;
-		if (msg_sys->msg_iovlen > UIO_MAXIOV)
-			goto out;
-		err = -ENOMEM;
-		iov = kmalloc(msg_sys->msg_iovlen * sizeof(struct iovec),
-			      GFP_KERNEL);
-		if (!iov)
-			goto out;
-	}
-
 	/* This will also move the address data into kernel space */
-	if (MSG_CMSG_COMPAT & flags) {
-		err = verify_compat_iovec(msg_sys, iov, &address, VERIFY_READ);
-	} else
-		err = verify_iovec(msg_sys, iov, &address, VERIFY_READ);
+	if (MSG_CMSG_COMPAT & flags)
+		err = verify_compat_iovec(msg_sys, iovstack, &address, WRITE);
+	else
+		err = verify_iovec(msg_sys, iovstack, &address, WRITE);
 	if (err < 0)
 		goto out_freeiov;
+	iov = msg_sys->msg_iov;
 	total_len = err;
 
 	err = -ENOBUFS;
@@ -2118,7 +2108,6 @@ out_freectl:
 out_freeiov:
 	if (iov != iovstack)
 		kfree(iov);
-out:
 	return err;
 }
 
@@ -2244,28 +2233,18 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
 			return err;
 	}
 
-	if (msg_sys->msg_iovlen > UIO_FASTIOV) {
-		err = -EMSGSIZE;
-		if (msg_sys->msg_iovlen > UIO_MAXIOV)
-			goto out;
-		err = -ENOMEM;
-		iov = kmalloc(msg_sys->msg_iovlen * sizeof(struct iovec),
-			      GFP_KERNEL);
-		if (!iov)
-			goto out;
-	}
-
 	/* Save the user-mode address (verify_iovec will change the
 	 * kernel msghdr to use the kernel address space)
 	 */
 	uaddr = (__force void __user *)msg_sys->msg_name;
 	uaddr_len = COMPAT_NAMELEN(msg);
 	if (MSG_CMSG_COMPAT & flags)
-		err = verify_compat_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
+		err = verify_compat_iovec(msg_sys, iovstack, &addr, READ);
 	else
-		err = verify_iovec(msg_sys, iov, &addr, VERIFY_WRITE);
+		err = verify_iovec(msg_sys, iovstack, &addr, READ);
 	if (err < 0)
 		goto out_freeiov;
+	iov = msg_sys->msg_iov;
 	total_len = err;
 
 	cmsg_ptr = (unsigned long)msg_sys->msg_control;
@@ -2306,7 +2285,6 @@ static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
 out_freeiov:
 	if (iov != iovstack)
 		kfree(iov);
-out:
 	return err;
 }
 
-- 
1.7.10.4

^ permalink raw reply related

* [PATCH 1/5] separate kernel- and userland-side msghdr
From: Al Viro @ 2014-11-18 19:41 UTC (permalink / raw)
  To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118194053.GA14641@ZenIV.linux.org.uk>

Kernel-side struct msghdr is (currently) using the same layout as
userland one, but it's not a one-to-one copy - even without considering
32bit compat issues, we have msg_iov, msg_name and msg_control copied
to kernel[1].  It's fairly localized, so we get away with a few functions
where that knowledge is needed (and we could shrink that set even
more).  Pretty much everything deals with the kernel-side variant and
the few places that want userland one just use a bunch of force-casts
to paper over the differences.

The thing is, kernel-side definition of struct msghdr is *not* exposed
in include/uapi - libc doesn't see it, etc.  So we can add struct user_msghdr,
with proper annotations and let the few places that ever deal with those
beasts use it for userland pointers.  Saner typechecking aside, that will
allow to change the layout of kernel-side msghdr - e.g. replace
msg_iov/msg_iovlen there with struct iov_iter, getting rid of the need
to modify the iovec as we copy data to/from it, etc.

We could introduce kernel_msghdr instead, but that would create much more
noise - the absolute majority of the instances would need to have the
type switched to kernel_msghdr and definition of struct msghdr in
include/linux/socket.h is not going to be seen by userland anyway.

This commit just introduces user_msghdr and switches the few places that
are dealing with userland-side msghdr to it.

[1] actually, it's even trickier than that - we copy msg_control for
sendmsg, but keep the userland address on recvmsg.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/arm/kernel/sys_oabi-compat.c |    4 ++--
 include/linux/socket.h            |   16 +++++++++++++---
 include/linux/syscalls.h          |    6 +++---
 net/compat.c                      |    4 ++--
 net/socket.c                      |   29 ++++++++++++++++-------------
 5 files changed, 36 insertions(+), 23 deletions(-)

diff --git a/arch/arm/kernel/sys_oabi-compat.c b/arch/arm/kernel/sys_oabi-compat.c
index e90a314..b83f3b7 100644
--- a/arch/arm/kernel/sys_oabi-compat.c
+++ b/arch/arm/kernel/sys_oabi-compat.c
@@ -400,7 +400,7 @@ asmlinkage long sys_oabi_sendto(int fd, void __user *buff,
 	return sys_sendto(fd, buff, len, flags, addr, addrlen);
 }
 
-asmlinkage long sys_oabi_sendmsg(int fd, struct msghdr __user *msg, unsigned flags)
+asmlinkage long sys_oabi_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags)
 {
 	struct sockaddr __user *addr;
 	int msg_namelen;
@@ -446,7 +446,7 @@ asmlinkage long sys_oabi_socketcall(int call, unsigned long __user *args)
 		break;
 	case SYS_SENDMSG:
 		if (copy_from_user(a, args, 3 * sizeof(long)) == 0)
-			r = sys_oabi_sendmsg(a[0], (struct msghdr __user *)a[1], a[2]);
+			r = sys_oabi_sendmsg(a[0], (struct user_msghdr __user *)a[1], a[2]);
 		break;
 	default:
 		r = sys_socketcall(call, args);
diff --git a/include/linux/socket.h b/include/linux/socket.h
index bb9b836..51bd666 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -53,10 +53,20 @@ struct msghdr {
 	__kernel_size_t	msg_controllen;	/* ancillary data buffer length */
 	unsigned int	msg_flags;	/* flags on received message */
 };
+ 
+struct user_msghdr {
+	void		__user *msg_name;	/* ptr to socket address structure */
+	int		msg_namelen;		/* size of socket address structure */
+	struct iovec	__user *msg_iov;	/* scatter/gather array */
+	__kernel_size_t	msg_iovlen;		/* # elements in msg_iov */
+	void		__user *msg_control;	/* ancillary data */
+	__kernel_size_t	msg_controllen;		/* ancillary data buffer length */
+	unsigned int	msg_flags;		/* flags on received message */
+};
 
 /* For recvmmsg/sendmmsg */
 struct mmsghdr {
-	struct msghdr   msg_hdr;
+	struct user_msghdr  msg_hdr;
 	unsigned int        msg_len;
 };
 
@@ -319,8 +329,8 @@ extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
 struct timespec;
 
 /* The __sys_...msg variants allow MSG_CMSG_COMPAT */
-extern long __sys_recvmsg(int fd, struct msghdr __user *msg, unsigned flags);
-extern long __sys_sendmsg(int fd, struct msghdr __user *msg, unsigned flags);
+extern long __sys_recvmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
+extern long __sys_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
 extern int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
 			  unsigned int flags, struct timespec *timeout);
 extern int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg,
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index bda9b81..c9afdc7 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -25,7 +25,7 @@ struct linux_dirent64;
 struct list_head;
 struct mmap_arg_struct;
 struct msgbuf;
-struct msghdr;
+struct user_msghdr;
 struct mmsghdr;
 struct msqid_ds;
 struct new_utsname;
@@ -601,13 +601,13 @@ asmlinkage long sys_getpeername(int, struct sockaddr __user *, int __user *);
 asmlinkage long sys_send(int, void __user *, size_t, unsigned);
 asmlinkage long sys_sendto(int, void __user *, size_t, unsigned,
 				struct sockaddr __user *, int);
-asmlinkage long sys_sendmsg(int fd, struct msghdr __user *msg, unsigned flags);
+asmlinkage long sys_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
 asmlinkage long sys_sendmmsg(int fd, struct mmsghdr __user *msg,
 			     unsigned int vlen, unsigned flags);
 asmlinkage long sys_recv(int, void __user *, size_t, unsigned);
 asmlinkage long sys_recvfrom(int, void __user *, size_t, unsigned,
 				struct sockaddr __user *, int __user *);
-asmlinkage long sys_recvmsg(int fd, struct msghdr __user *msg, unsigned flags);
+asmlinkage long sys_recvmsg(int fd, struct user_msghdr __user *msg, unsigned flags);
 asmlinkage long sys_recvmmsg(int fd, struct mmsghdr __user *msg,
 			     unsigned int vlen, unsigned flags,
 			     struct timespec __user *timeout);
diff --git a/net/compat.c b/net/compat.c
index bc8aeef..562e920 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -740,7 +740,7 @@ COMPAT_SYSCALL_DEFINE3(sendmsg, int, fd, struct compat_msghdr __user *, msg, uns
 {
 	if (flags & MSG_CMSG_COMPAT)
 		return -EINVAL;
-	return __sys_sendmsg(fd, (struct msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
+	return __sys_sendmsg(fd, (struct user_msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
 }
 
 COMPAT_SYSCALL_DEFINE4(sendmmsg, int, fd, struct compat_mmsghdr __user *, mmsg,
@@ -756,7 +756,7 @@ COMPAT_SYSCALL_DEFINE3(recvmsg, int, fd, struct compat_msghdr __user *, msg, uns
 {
 	if (flags & MSG_CMSG_COMPAT)
 		return -EINVAL;
-	return __sys_recvmsg(fd, (struct msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
+	return __sys_recvmsg(fd, (struct user_msghdr __user *)msg, flags | MSG_CMSG_COMPAT);
 }
 
 COMPAT_SYSCALL_DEFINE4(recv, int, fd, void __user *, buf, compat_size_t, len, unsigned int, flags)
diff --git a/net/socket.c b/net/socket.c
index fe20c31..0ae8147 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -1989,8 +1989,11 @@ struct used_address {
 };
 
 static int copy_msghdr_from_user(struct msghdr *kmsg,
-				 struct msghdr __user *umsg)
+				 struct user_msghdr __user *umsg)
 {
+	/* We are relying on the (currently) identical layouts.  Once
+	 * the kernel-side changes, this place will need to be updated
+	 */
 	if (copy_from_user(kmsg, umsg, sizeof(struct msghdr)))
 		return -EFAULT;
 
@@ -2005,7 +2008,7 @@ static int copy_msghdr_from_user(struct msghdr *kmsg,
 	return 0;
 }
 
-static int ___sys_sendmsg(struct socket *sock, struct msghdr __user *msg,
+static int ___sys_sendmsg(struct socket *sock, struct user_msghdr __user *msg,
 			 struct msghdr *msg_sys, unsigned int flags,
 			 struct used_address *used_address)
 {
@@ -2123,7 +2126,7 @@ out:
  *	BSD sendmsg interface
  */
 
-long __sys_sendmsg(int fd, struct msghdr __user *msg, unsigned flags)
+long __sys_sendmsg(int fd, struct user_msghdr __user *msg, unsigned flags)
 {
 	int fput_needed, err;
 	struct msghdr msg_sys;
@@ -2140,7 +2143,7 @@ out:
 	return err;
 }
 
-SYSCALL_DEFINE3(sendmsg, int, fd, struct msghdr __user *, msg, unsigned int, flags)
+SYSCALL_DEFINE3(sendmsg, int, fd, struct user_msghdr __user *, msg, unsigned int, flags)
 {
 	if (flags & MSG_CMSG_COMPAT)
 		return -EINVAL;
@@ -2177,7 +2180,7 @@ int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
 
 	while (datagrams < vlen) {
 		if (MSG_CMSG_COMPAT & flags) {
-			err = ___sys_sendmsg(sock, (struct msghdr __user *)compat_entry,
+			err = ___sys_sendmsg(sock, (struct user_msghdr __user *)compat_entry,
 					     &msg_sys, flags, &used_address);
 			if (err < 0)
 				break;
@@ -2185,7 +2188,7 @@ int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
 			++compat_entry;
 		} else {
 			err = ___sys_sendmsg(sock,
-					     (struct msghdr __user *)entry,
+					     (struct user_msghdr __user *)entry,
 					     &msg_sys, flags, &used_address);
 			if (err < 0)
 				break;
@@ -2215,7 +2218,7 @@ SYSCALL_DEFINE4(sendmmsg, int, fd, struct mmsghdr __user *, mmsg,
 	return __sys_sendmmsg(fd, mmsg, vlen, flags);
 }
 
-static int ___sys_recvmsg(struct socket *sock, struct msghdr __user *msg,
+static int ___sys_recvmsg(struct socket *sock, struct user_msghdr __user *msg,
 			 struct msghdr *msg_sys, unsigned int flags, int nosec)
 {
 	struct compat_msghdr __user *msg_compat =
@@ -2311,7 +2314,7 @@ out:
  *	BSD recvmsg interface
  */
 
-long __sys_recvmsg(int fd, struct msghdr __user *msg, unsigned flags)
+long __sys_recvmsg(int fd, struct user_msghdr __user *msg, unsigned flags)
 {
 	int fput_needed, err;
 	struct msghdr msg_sys;
@@ -2328,7 +2331,7 @@ out:
 	return err;
 }
 
-SYSCALL_DEFINE3(recvmsg, int, fd, struct msghdr __user *, msg,
+SYSCALL_DEFINE3(recvmsg, int, fd, struct user_msghdr __user *, msg,
 		unsigned int, flags)
 {
 	if (flags & MSG_CMSG_COMPAT)
@@ -2373,7 +2376,7 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
 		 * No need to ask LSM for more than the first datagram.
 		 */
 		if (MSG_CMSG_COMPAT & flags) {
-			err = ___sys_recvmsg(sock, (struct msghdr __user *)compat_entry,
+			err = ___sys_recvmsg(sock, (struct user_msghdr __user *)compat_entry,
 					     &msg_sys, flags & ~MSG_WAITFORONE,
 					     datagrams);
 			if (err < 0)
@@ -2382,7 +2385,7 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
 			++compat_entry;
 		} else {
 			err = ___sys_recvmsg(sock,
-					     (struct msghdr __user *)entry,
+					     (struct user_msghdr __user *)entry,
 					     &msg_sys, flags & ~MSG_WAITFORONE,
 					     datagrams);
 			if (err < 0)
@@ -2571,13 +2574,13 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
 				   (int __user *)a[4]);
 		break;
 	case SYS_SENDMSG:
-		err = sys_sendmsg(a0, (struct msghdr __user *)a1, a[2]);
+		err = sys_sendmsg(a0, (struct user_msghdr __user *)a1, a[2]);
 		break;
 	case SYS_SENDMMSG:
 		err = sys_sendmmsg(a0, (struct mmsghdr __user *)a1, a[2], a[3]);
 		break;
 	case SYS_RECVMSG:
-		err = sys_recvmsg(a0, (struct msghdr __user *)a1, a[2]);
+		err = sys_recvmsg(a0, (struct user_msghdr __user *)a1, a[2]);
 		break;
 	case SYS_RECVMMSG:
 		err = sys_recvmmsg(a0, (struct mmsghdr __user *)a1, a[2], a[3],
-- 
1.7.10.4

^ permalink raw reply related

* [patches][RFC] situation with csum_and_copy_... API
From: Al Viro @ 2014-11-18 19:40 UTC (permalink / raw)
  To: netdev; +Cc: Linus Torvalds, David Miller, linux-kernel
In-Reply-To: <20141118084745.GT7996@ZenIV.linux.org.uk>

On Tue, Nov 18, 2014 at 08:47:45AM +0000, Al Viro wrote:

> I do have a patch doing just that; the question is what to do with csum-and-copy
> primitives.  Originally I planned to simply strip those access_ok() from those
> (both the explicit calls and use of copy_from_user() where we ought to use
> __copy_from_user(), etc.), but that's not nice to potential out-of-tree callers
> of those suckers.  If any of those exist and manage to cope with the wonderful
> calling conventions, that is.  As it is, we have the total of 4 callers of
> csum_and_copy_from_user() and 2 callers of csum_and_copy_to_user(), all in
> networking code.  Do we care about potential out-of-tree users existing and
> getting screwed by such change?  Davem, Linus?

FWIW, the beginning of series in question follows; removal of those
access_ok() is 3/5.  The series is longer than that (see vfs.git#iov_iter-net
for a bit more, and there's more stuff in local queue still too much in flux
to push them out), but all the stuff relevant to validating iovecs on
sendmsg/recvmsg and getting rid of excessive access_ok() is in the first 5
commits.

^ permalink raw reply

* loan test
From: Frank.Laurent @ 2014-11-18 19:33 UTC (permalink / raw)
  To: netdev

NEED LOAN?

We offer our Loans in the following categories. 
Auto Loans Mortgage loans Business Loans Personal Loans Real Estate Loan. 

For more information and Application,Contact our representative with the information below,  

Loan Agent:  Donald Gotze  
Email: gotzedonald@yahoo.com    

Thank you for your response.  
Yours Faithfully,
Frank Laurent

^ permalink raw reply

* [PATCH] ixgbe: Correctly disable vlan filter in promiscuous mode
From: Vladislav Yasevich @ 2014-11-18 19:28 UTC (permalink / raw)
  To: netdev; +Cc: Vladislav Yasevich, Jeff Kirsher, Jacob Keller

IXGBE adapater seems to require that vlan filtering be enabled if
VMDQ
or SRIOV are enabled.  When those functions are disabled,
vlan filtering may be disabled in promiscuous mode.

Prior to commit a9b8943ee129e11045862d6d6e25c5b63c95403c
    ixgbe: remove vlan_filter_disable and enable functions

the logic was correct.  However, after the commit the logic
got reversed and vlan filtered in now turned on when VMDQ/SRIOV
is disabled.

This patch changes the condition to enable hw vlan filtered
when VMDQ or SRIOV is enabled.

Fixes: a9b8943ee129e11045862d6d6e25c5b63c95403c (ixgbe: remove
vlan_filter_disable and enable functions)
CC: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
CC: Jacob Keller <jacob.e.keller@intel.com>
Signed-off-by: Vladislav Yasevich <vyasevic@redhat.com>
---
 drivers/net/ethernet/intel/ixgbe/ixgbe_main.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
index d2df4e3..3f81c7a 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_main.c
@@ -3936,8 +3936,8 @@ void ixgbe_set_rx_mode(struct net_device *netdev)
 		 * if SR-IOV and VMDQ are disabled - otherwise ensure
 		 * that hardware VLAN filters remain enabled.
 		 */
-		if (!(adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
-					IXGBE_FLAG_SRIOV_ENABLED)))
+		if (adapter->flags & (IXGBE_FLAG_VMDQ_ENABLED |
+				      IXGBE_FLAG_SRIOV_ENABLED))
 			vlnctrl |= (IXGBE_VLNCTRL_VFE | IXGBE_VLNCTRL_CFIEN);
 	} else {
 		if (netdev->flags & IFF_ALLMULTI) {
-- 
1.9.3

^ permalink raw reply related

* [PATCH 1/1] net: pktgen: Deletion of an unnecessary check before the function call "proc_remove"
From: SF Markus Elfring @ 2014-11-18 19:16 UTC (permalink / raw)
  To: David S. Miller, netdev; +Cc: LKML, kernel-janitors, Coccinelle
In-Reply-To: <5317A59D.4@users.sourceforge.net>

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Tue, 18 Nov 2014 20:10:34 +0100

The proc_remove() function tests whether its argument is NULL and then
returns immediately. Thus the test around the call is not needed.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 net/core/pktgen.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/net/core/pktgen.c b/net/core/pktgen.c
index 8b849dd..35046a8 100644
--- a/net/core/pktgen.c
+++ b/net/core/pktgen.c
@@ -3698,8 +3698,7 @@ static int pktgen_remove_device(struct pktgen_thread *t,
 	/* Remove proc before if_list entry, because add_device uses
 	 * list to determine if interface already exist, avoid race
 	 * with proc_create_data() */
-	if (pkt_dev->entry)
-		proc_remove(pkt_dev->entry);
+	proc_remove(pkt_dev->entry);
 
 	/* And update the thread if_list */
 	_rem_dev_from_if_list(t, pkt_dev);
-- 
2.1.3

^ 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