* Re: [PATCH] net/ipv6/exthdrs.c et al: Optional strict PadN option checking
From: Eldad Zack @ 2012-04-07 15:13 UTC (permalink / raw)
To: David Miller; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <CABe5r8KL=c1=emU_20ODwM5OvFJS9vaKV1ux2hSmGB5tmZoJ1w@mail.gmail.com>
(resend, previous message was [rightfully] rejected by the mailer)
On 7 April 2012 17:09, Eldad Zack <eldad@fogrefinery.com> wrote:
>
> On 7 April 2012 14:46, David Miller <davem@davemloft.net> wrote:
>>
>> From: Eldad Zack <eldad@fogrefinery.com>
>> Date: Sat, 7 Apr 2012 12:44:27 +0200
>>
>> > Added strict checking of PadN, which can be used to increase header
>> > size and thus push the protocol header into the 2nd fragment.
>>
>> A sysctl _AND_ a kconfig option, give me a break, that's
>> pointless.
>>
>> Either we want this or we don't.
>
>
> You're right, that kconfig bit was silly.
> I'd like to keep the sysctl knob, though.
>
> Fixed patch follows.
>
> Eldad
^ permalink raw reply
* Re: Pull request for 'davem-next.base_addr.removal' branch
From: David Miller @ 2012-04-07 13:28 UTC (permalink / raw)
To: romieu
Cc: netdev, gallatin, andy, chris.snook, venza, DavidLv,
grantgrundler, ionut, jcliburn, jdmason, mason, mchan,
stas.yakovlev, steve.glendinning, thockin
In-Reply-To: <20120407.085012.658444364459389098.davem@davemloft.net>
From: David Miller <davem@davemloft.net>
Date: Sat, 07 Apr 2012 08:50:12 -0400 (EDT)
> From: Francois Romieu <romieu@fr.zoreil.com>
> Date: Sat, 7 Apr 2012 12:20:51 +0200
>
>> Please pull from branch 'davem-next.base_addr.removal' in repository
>>
>> git://violet.fr.zoreil.com/romieu/linux davem-next.base_addr.removal
>>
>> to get the changes below.
>
> Pulled, thanks for doing this work Francois.
I had to add the following patch to fix a warning:
--------------------
fealnx: Remove unused local label 'out' in netdev_open().
Signed-off-by: David S. Miller <davem@davemloft.net>
---
drivers/net/ethernet/fealnx.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c
index 6f939a1..9d71c9c 100644
--- a/drivers/net/ethernet/fealnx.c
+++ b/drivers/net/ethernet/fealnx.c
@@ -923,7 +923,6 @@ static int netdev_open(struct net_device *dev)
np->reset_timer.data = (unsigned long) dev;
np->reset_timer.function = reset_timer;
np->reset_timer_armed = 0;
-out:
return rc;
}
--
1.7.7.6
^ permalink raw reply related
* Re: Pull request for 'davem-next.base_addr.removal' branch
From: David Miller @ 2012-04-07 12:50 UTC (permalink / raw)
To: romieu
Cc: netdev, gallatin, andy, chris.snook, venza, DavidLv,
grantgrundler, ionut, jcliburn, jdmason, mason, mchan,
stas.yakovlev, steve.glendinning, thockin
In-Reply-To: <20120407102051.GA5551@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Sat, 7 Apr 2012 12:20:51 +0200
> Please pull from branch 'davem-next.base_addr.removal' in repository
>
> git://violet.fr.zoreil.com/romieu/linux davem-next.base_addr.removal
>
> to get the changes below.
Pulled, thanks for doing this work Francois.
^ permalink raw reply
* Re: [PATCH] net/ipv6/exthdrs.c et al: Optional strict PadN option checking
From: David Miller @ 2012-04-07 12:46 UTC (permalink / raw)
To: eldad; +Cc: kuznet, jmorris, yoshfuji, kaber, netdev, linux-kernel
In-Reply-To: <1333795467-3643-1-git-send-email-eldad@fogrefinery.com>
From: Eldad Zack <eldad@fogrefinery.com>
Date: Sat, 7 Apr 2012 12:44:27 +0200
> Added strict checking of PadN, which can be used to increase header
> size and thus push the protocol header into the 2nd fragment.
>
> PadN is used to align the options within the Hop-by-Hop or
> Destination Options header to 64-bit boundaries. The maximum valid
> size is thus 7 bytes.
> RFC 4942 recommends to actively check the "payload" itself and
> ensure that it contains only zeroes.
>
> See also RFC 4942 section 2.1.9.5.
>
> Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
A sysctl _AND_ a kconfig option, give me a break, that's
pointless.
Either we want this or we don't.
^ permalink raw reply
* [PATCH V2 net-next 1/1] e100: enable transmit time stamping.
From: Richard Cochran @ 2012-04-07 12:49 UTC (permalink / raw)
To: netdev
Cc: David Miller, Jeff Kirsher, Alex Duyck, Bruce Allan,
Carolyn Wyborny, Don Skidmore, Greg Rose, Jesse Brandeburg,
John Ronciak, Peter P Waskiewicz Jr
In-Reply-To: <cover.1333802620.git.richardcochran@gmail.com>
This patch enables software (and phy device) transmit time stamping.
Tested on an old PIII laptop with built in NIC.
Cc: Alex Duyck <alexander.h.duyck@intel.com>
Cc: Bruce Allan <bruce.w.allan@intel.com>
Cc: Carolyn Wyborny <carolyn.wyborny@intel.com>
Cc: Don Skidmore <donald.c.skidmore@intel.com>
Cc: Greg Rose <gregory.v.rose@intel.com>
Cc: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: John Ronciak <john.ronciak@intel.com>
Cc: Peter P Waskiewicz Jr <peter.p.waskiewicz.jr@intel.com>
Signed-off-by: Richard Cochran <richardcochran@gmail.com>
---
drivers/net/ethernet/intel/e100.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/drivers/net/ethernet/intel/e100.c b/drivers/net/ethernet/intel/e100.c
index e498eff..872e17c 100644
--- a/drivers/net/ethernet/intel/e100.c
+++ b/drivers/net/ethernet/intel/e100.c
@@ -1759,6 +1759,7 @@ static void e100_xmit_prepare(struct nic *nic, struct cb *cb,
skb->data, skb->len, PCI_DMA_TODEVICE));
/* check for mapping failure? */
cb->u.tcb.tbd.size = cpu_to_le16(skb->len);
+ skb_tx_timestamp(skb);
}
static netdev_tx_t e100_xmit_frame(struct sk_buff *skb,
--
1.7.2.5
^ permalink raw reply related
* [PATCH V2 net-next 0/1] e100: support Tx timestamping
From: Richard Cochran @ 2012-04-07 12:49 UTC (permalink / raw)
To: netdev; +Cc: David Miller, Jeff Kirsher
In-Reply-To: <1333532088.3046.22.camel@jtkirshe-mobl>
Changes in V2:
- Avoid time stamping of firmware blobs
This patch adds SO_TIMESTAMPING in the transmit path for the Ethernet
MACs found in my older laptop.
Richard Cochran (1):
e100: enable transmit time stamping.
drivers/net/ethernet/intel/e100.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
--
1.7.2.5
^ permalink raw reply
* [PATCH] net/ipv6/exthdrs.c et al: Optional strict PadN option checking
From: Eldad Zack @ 2012-04-07 10:44 UTC (permalink / raw)
To: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, atrick McHardy
Cc: netdev, linux-kernel, Eldad Zack
Added strict checking of PadN, which can be used to increase header
size and thus push the protocol header into the 2nd fragment.
PadN is used to align the options within the Hop-by-Hop or
Destination Options header to 64-bit boundaries. The maximum valid
size is thus 7 bytes.
RFC 4942 recommends to actively check the "payload" itself and
ensure that it contains only zeroes.
See also RFC 4942 section 2.1.9.5.
Signed-off-by: Eldad Zack <eldad@fogrefinery.com>
---
Documentation/networking/ip-sysctl.txt | 7 +++++++
include/net/ipv6.h | 4 ++++
net/ipv6/Kconfig | 14 ++++++++++++++
net/ipv6/exthdrs.c | 22 ++++++++++++++++++++++
net/ipv6/sysctl_net_ipv6.c | 7 +++++++
5 files changed, 54 insertions(+)
diff --git a/Documentation/networking/ip-sysctl.txt b/Documentation/networking/ip-sysctl.txt
index ad3e80e..abaa1cf 100644
--- a/Documentation/networking/ip-sysctl.txt
+++ b/Documentation/networking/ip-sysctl.txt
@@ -1007,6 +1007,13 @@ bindv6only - BOOLEAN
Default: FALSE (as specified in RFC3493)
+padn_strict - BOOLEAN
+ Perform strict checking of PadN.
+ TRUE: enable strict PadN checking
+ FALSE: disable strict PadN checking
+
+ Default: FALSE
+
IPv6 Fragmentation:
ip6frag_high_thresh - INTEGER
diff --git a/include/net/ipv6.h b/include/net/ipv6.h
index e4170a2..1458915 100644
--- a/include/net/ipv6.h
+++ b/include/net/ipv6.h
@@ -115,6 +115,10 @@ struct frag_hdr {
extern int sysctl_mld_max_msf;
extern struct ctl_path net_ipv6_ctl_path[];
+#if defined(CONFIG_IPV6_PADN_STRICT)
+extern int sysctl_padn_strict;
+#endif
+
#define _DEVINC(net, statname, modifier, idev, field) \
({ \
struct inet6_dev *_idev = (idev); \
diff --git a/net/ipv6/Kconfig b/net/ipv6/Kconfig
index 36d7437..a10a684 100644
--- a/net/ipv6/Kconfig
+++ b/net/ipv6/Kconfig
@@ -250,4 +250,18 @@ config IPV6_PIMSM_V2
Support for IPv6 PIM multicast routing protocol PIM-SMv2.
If unsure, say N.
+config IPV6_PADN_STRICT
+ bool "IPv6: Strict PadN processing"
+ ---help---
+ This enables strict processing of PadN (See RFC 4942, Section
+ 2.1.9.5). Any packet with PadN exceeding the length of 7 is
+ dropped, as well as with packets with non-zero PadN payload.
+
+ By default the kernel does not perform these checks.
+ To enable them, use the following command:
+
+ echo 1 >/proc/sys/net/ipv6/padn_strict
+
+ If unsure, say N.
+
endif # IPV6
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index c486b8e..e04d4ff 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -49,6 +49,10 @@
#include <asm/uaccess.h>
+#if defined(CONFIG_IPV6_PADN_STRICT)
+int sysctl_padn_strict;
+#endif
+
int ipv6_find_tlv(struct sk_buff *skb, int offset, int type)
{
const unsigned char *nh = skb_network_header(skb);
@@ -160,7 +164,25 @@ static int ip6_parse_tlv(struct tlvtype_proc *procs, struct sk_buff *skb)
break;
case IPV6_TLV_PADN:
+#if defined(CONFIG_IPV6_PADN_STRICT)
+ if (sysctl_padn_strict) {
+ int i;
+ /* RFC 2460 states that the purpose of PadN is
+ to align the containing header to multiples
+ of 8. 7 is therefore the highest valid value.
+ See also RFC 4942, Section 2.1.9.5.*/
+ if (optlen > 7)
+ goto bad;
+ /* RFC 4942 recommends receiving hosts to
+ actively check PadN payload to contain
+ only zeroes. */
+ for (i = 2; i < optlen; i++) {
+ if (nh[off + i] != 0)
+ goto bad;
+ }
+ }
break;
+#endif
default: /* Other TLV code so scan list */
if (optlen > len)
diff --git a/net/ipv6/sysctl_net_ipv6.c b/net/ipv6/sysctl_net_ipv6.c
index 166a57c..c7f07f9 100644
--- a/net/ipv6/sysctl_net_ipv6.c
+++ b/net/ipv6/sysctl_net_ipv6.c
@@ -48,6 +48,13 @@ static ctl_table ipv6_table_template[] = {
.mode = 0644,
.proc_handler = proc_dointvec
},
+ {
+ .procname = "padn_strict",
+ .data = &sysctl_padn_strict,
+ .maxlen = sizeof(int),
+ .mode = 0644,
+ .proc_handler = proc_dointvec
+ },
{ }
};
--
1.7.9.5
^ permalink raw reply related
* Pull request for 'davem-next.base_addr.removal' branch
From: Francois Romieu @ 2012-04-07 10:20 UTC (permalink / raw)
To: David Miller
Cc: netdev, Andrew Gallatin, Andy Gospodarek, Chris Snook,
Daniele Venzano, David Lv, Grant Grundler, Ion Badulescu,
Jay Cliburn, Jon Mason, Jon Mason, Michael Chan,
Stanislav Yakovlev, Steve Glendinning, Tim Hockin
Please pull from branch 'davem-next.base_addr.removal' in repository
git://violet.fr.zoreil.com/romieu/linux davem-next.base_addr.removal
to get the changes below.
Changes since #2 (2012/04/06)
- reverted the request_irq tests for '< 0' to their original state
(see patch below)
- added Acked-by from Michael Chan, Grant Grundler, Andrew Gallatin
Changes since #1 (2012/03/15)
- display the PCI resource address in the bnx2 driver as suggested by
Michael Chan. It is done as well where things have been moved to mapped
addresses, namely sc92031, epic100 and rrunner. I have not done it where
net_device.base_addr already contained a mapped address.
- convert drivers to io{read/write} in place of inl/out #define. Minimal
comments keep trace of the nature of the committed PCI bar range when it
gets remapped. It applies to dl2k, dmfe, epic100, uli526x, xircom_cb
- reworked the chip dependent uli526x phy operations to minimize conditional
registers offsets and parameters. void (__iomem) * are too easy to abuse.
- added myri10ge, rrunner, ipw2100, ipw2200
Few files changed: a diff from the version #2 is included below.
Previous branch has been moved to davem-next.base_addr.removal-20120406.
I should be able to address Grant's suggestions for further changes this
week end after some r8169 work.
Distance from 'davem-next' (36b7777590be33567be50db7f82282c3237428ad)
---------------------------------------------------------------------
019d077a530d2c6d6032ddd92a91520c3479ff05
9b717075e96daed94a10b4b0aaeb88c4c4bb0da3
a3442794c5b7925ef240e8f5ce7683345f5cc84d
0193fc5efd95c18bed1d03c57b2f916906662753
a74254588754bf7bc5c60f2bcc9ee5f66b749ea2
57d6d456cfb89264f87d24f52640ede23fdf12bd
5820e97a299e502e71dd5587ed2bf63a75d4f4f7
aae9bc302de493ad62102e7cdb1e123648057b66
3acf4b5cde8123a7a243d9dcd63f3e6990c8e5bb
5e3cc4e3aaeae953c224bbe92f0ea8d90dfb1b63
65712ec016788538d27c0b0452e57b751776914e
d710ce135731c101b6fc131c07f3db0cdb0d95fd
b5a80837b7e125729a49b2a8b80558d09bea7e19
308f2888a3ff442167e2aea419225445b7a1b8b6
ebaf7f8f78e8600b56010121766ed832f2f57b0f
d59a1881c0ff63f9edbc7f4ad5a5e593fe779e1b
5e58deb917bfa8dad71cb2e2b9f9572746ff8e6a
7deb1182175ee06cfcea40452d4f2fb1e33fcbb8
a173460a6391ed5c38b63bcaedc6afc30f4de3b2
480c28642d794589b1279c06b9ef60f4a63acb59
c4a9f0854bbea281f3d332c8c2b2b65a67b11616
c0bd55efd7b7ea8346ec3b5ce8414f978fede1f5
c0357e975afdbbedab5c662d19bef865f02adc17
2d5fb6283cb921a1f66454f8a603aaa973cbc24b
ea8f2ed0f1b943e499e760feced4038f95b2b71f
0c18acc1ed57d54195427d16316d5ea09d840b0e
a69afe3263717ba9384cf18d05722c598f6820af
80777c54d2a6ebeff783b638e92fba40f0881ba5
b5efab99ed92dd3b432e2f414d3979e8c2acd382
436dfc461b87b49d67867a798c91cd8cfff23f12
32e819e46e028ec5901048baee66775d71c4ec51
c514f285c37fc705e59956ae0b9fc79d28905d3b
dfda3578867bbfa35c629b58b5886dd9f5da11ca
0ca0aa08eb70dd23c1bc5d154c3d8ba157b712e9
05d334eca9994680a6cb8fba3f19955356ccf72a
93f7fab433606a0ee6153788213de2a822736322
03a2384e68f8cd464320c79c8c8c9f75f7172fa5
9195182d82890c270819880695f491fcb2ac4ca2
4ee39fd1a97115f537664cf18dc85bda916c24e5
Diffstat
--------
drivers/net/ethernet/adaptec/starfire.c | 54 +--
drivers/net/ethernet/atheros/atl1c/atl1c_main.c | 5 +-
drivers/net/ethernet/atheros/atl1e/atl1e_main.c | 17 +-
drivers/net/ethernet/broadcom/bnx2.c | 41 +--
drivers/net/ethernet/dec/tulip/de2104x.c | 34 +-
drivers/net/ethernet/dec/tulip/dmfe.c | 295 ++++++++-------
drivers/net/ethernet/dec/tulip/tulip_core.c | 27 +-
drivers/net/ethernet/dec/tulip/uli526x.c | 443 +++++++++++-----------
drivers/net/ethernet/dec/tulip/winbond-840.c | 17 +-
drivers/net/ethernet/dec/tulip/xircom_cb.c | 280 ++++++++------
drivers/net/ethernet/dlink/dl2k.c | 416 ++++++++++----------
drivers/net/ethernet/dlink/dl2k.h | 19 +-
drivers/net/ethernet/dlink/sundance.c | 12 +-
drivers/net/ethernet/fealnx.c | 15 +-
drivers/net/ethernet/myricom/myri10ge/myri10ge.c | 7 +-
drivers/net/ethernet/natsemi/natsemi.c | 67 ++--
drivers/net/ethernet/neterion/s2io.c | 14 +-
drivers/net/ethernet/neterion/vxge/vxge-main.c | 18 +-
drivers/net/ethernet/nvidia/forcedeth.c | 5 -
drivers/net/ethernet/packetengines/hamachi.c | 11 +-
drivers/net/ethernet/packetengines/yellowfin.c | 32 +-
drivers/net/ethernet/realtek/8139cp.c | 21 +-
drivers/net/ethernet/realtek/8139too.c | 136 +++----
drivers/net/ethernet/silan/sc92031.c | 34 +--
drivers/net/ethernet/sis/sis190.c | 26 +-
drivers/net/ethernet/sis/sis900.c | 375 ++++++++++---------
drivers/net/ethernet/smsc/epic100.c | 403 ++++++++++----------
drivers/net/ethernet/smsc/smsc9420.c | 47 ++--
drivers/net/ethernet/sun/sungem.c | 2 -
drivers/net/ethernet/sun/sunhme.c | 18 +-
drivers/net/ethernet/sun/sunhme.h | 1 +
drivers/net/ethernet/tehuti/tehuti.c | 4 -
drivers/net/ethernet/via/via-rhine.c | 12 +-
drivers/net/ethernet/via/via-velocity.c | 9 +-
drivers/net/hippi/rrunner.c | 82 ++---
drivers/net/wireless/ipw2x00/ipw2100.c | 131 +++----
drivers/net/wireless/ipw2x00/ipw2100.h | 1 +
drivers/net/wireless/ipw2x00/ipw2200.c | 4 -
38 files changed, 1525 insertions(+), 1610 deletions(-)
Shortlog
--------
Francois Romieu (39):
sungem: stop using net_device.{base_addr, irq}.
tehuti: stop using net_device.{base_addr, irq}.
forcedeth: stop using net_device.{base_addr, irq}.
atl1c: stop using net_device.{base_addr, irq}.
via-rhine: stop using net_device.{base_addr, irq}.
hamachi: stop using net_device.{base_addr, irq}.
via-velocity: stop using net_device.{base_addr, irq}.
sundance: stop using net_device.{base_addr, irq}.
vxge: stop using net_device.{base_addr, irq}.
fealnx: stop using net_device.{base_addr, irq}.
atl1e: stop using net_device.{base_addr, irq}.
s2io: stop using net_device.{base_addr, irq}.
8139cp: stop using net_device.{base_addr, irq}.
yellowfin: stop using net_device.{base_addr, irq}.
starfire: stop using net_device.{base_addr, irq}.
starfire: remove deprecated options.
bnx2: stop using net_device.{base_addr, irq}.
winbond840: stop using net_device.{base_addr, irq}.
sc92031: stop using net_device.{base_addr, irq}
sis190: stop using net_device.{base_addr, irq}
tulip_core: stop using net_device.{base_addr, irq}.
sunhme: stop using net_device.{base_addr, irq}.
uli526x: fix regions leak in driver probe error path.
xircom_cb: fix device probe error path.
xircom_cb: stop using net_device.{base_addr, irq} and convert to __iomem.
de2104x: stop using net_device.{base_addr, irq}.
smsc9420: stop using net_device.{base_addr, irq}.
natsemi: stop using net_device.{base_addr, irq}.
8139too: dev->{base_addr, irq} removal.
dl2k: stop using net_device.{base_addr, irq} and convert to __iomem.
uli526x: stop using net_device.{base_addr, irq} and convert to __iomem.
epic100: stop using net_device.{base_addr, irq} and convert to __iomem.
dmfe: stop using net_device.{base_addr, irq} and convert to __iomem.
sis900: stop using net_device.{base_addr, irq} and convert to __iomem.
myri10ge: stop using net_device.{base_addr, irq}.
rrunner: stop using net_device.{base_addr, irq}.
ipw2200: stop using net_device.{base_addr, irq}.
ipw2100: stop using net_device.base_addr.
ipw2100: remove useless tests in the PCI device remove path.
Patch from version #2
---------------------
diff --git a/drivers/net/ethernet/dec/tulip/tulip_core.c b/drivers/net/ethernet/dec/tulip/tulip_core.c
index 25fe117..c4f37ac 100644
--- a/drivers/net/ethernet/dec/tulip/tulip_core.c
+++ b/drivers/net/ethernet/dec/tulip/tulip_core.c
@@ -1902,7 +1902,7 @@ static int tulip_resume(struct pci_dev *pdev)
retval = request_irq(pdev->irq, tulip_interrupt, IRQF_SHARED,
dev->name, dev);
- if (retval < 0) {
+ if (retval) {
pr_err("request_irq failed in resume\n");
return retval;
}
diff --git a/drivers/net/ethernet/fealnx.c b/drivers/net/ethernet/fealnx.c
index 328c631..6f939a1 100644
--- a/drivers/net/ethernet/fealnx.c
+++ b/drivers/net/ethernet/fealnx.c
@@ -835,8 +835,8 @@ static int netdev_open(struct net_device *dev)
iowrite32(0x00000001, ioaddr + BCR); /* Reset */
rc = request_irq(irq, intr_handler, IRQF_SHARED, dev->name, dev);
- if (rc < 0)
- goto out;
+ if (rc)
+ return -EAGAIN;
for (i = 0; i < 3; i++)
iowrite16(((unsigned short*)dev->dev_addr)[i],
diff --git a/drivers/net/ethernet/packetengines/yellowfin.c b/drivers/net/ethernet/packetengines/yellowfin.c
index cf5f54b..04e622f 100644
--- a/drivers/net/ethernet/packetengines/yellowfin.c
+++ b/drivers/net/ethernet/packetengines/yellowfin.c
@@ -574,8 +574,8 @@ static int yellowfin_open(struct net_device *dev)
iowrite32(0x80000000, ioaddr + DMACtrl);
rc = request_irq(irq, yellowfin_interrupt, IRQF_SHARED, dev->name, dev);
- if (rc < 0)
- goto out;
+ if (rc)
+ return rc;
rc = yellowfin_init_ring(dev);
if (rc < 0)
diff --git a/drivers/net/ethernet/smsc/epic100.c b/drivers/net/ethernet/smsc/epic100.c
index 1710b4b..d01e59c 100644
--- a/drivers/net/ethernet/smsc/epic100.c
+++ b/drivers/net/ethernet/smsc/epic100.c
@@ -657,7 +657,7 @@ static int epic_open(struct net_device *dev)
napi_enable(&ep->napi);
rc = request_irq(irq, epic_interrupt, IRQF_SHARED, dev->name, dev);
- if (rc < 0) {
+ if (rc) {
napi_disable(&ep->napi);
return rc;
}
diff --git a/drivers/net/ethernet/smsc/smsc9420.c b/drivers/net/ethernet/smsc/smsc9420.c
index 292753f..fd33b21 100644
--- a/drivers/net/ethernet/smsc/smsc9420.c
+++ b/drivers/net/ethernet/smsc/smsc9420.c
@@ -1361,8 +1361,9 @@ static int smsc9420_open(struct net_device *dev)
result = request_irq(irq, smsc9420_isr, IRQF_SHARED | IRQF_DISABLED,
DRV_NAME, pd);
- if (result < 0) {
+ if (result) {
smsc_warn(IFUP, "Unable to use IRQ = %d", irq);
+ result = -ENODEV;
goto out_0;
}
diff --git a/drivers/net/ethernet/sun/sunhme.c b/drivers/net/ethernet/sun/sunhme.c
index a470f38..afb19a6 100644
--- a/drivers/net/ethernet/sun/sunhme.c
+++ b/drivers/net/ethernet/sun/sunhme.c
@@ -2185,12 +2185,12 @@ static int happy_meal_open(struct net_device *dev)
if ((hp->happy_flags & (HFLAG_QUATTRO|HFLAG_PCI)) != HFLAG_QUATTRO) {
res = request_irq(hp->irq, happy_meal_interrupt, IRQF_SHARED,
dev->name, dev);
- if (res < 0) {
+ if (res) {
HMD(("EAGAIN\n"));
printk(KERN_ERR "happy_meal(SBUS): Can't order irq %d to go.\n",
hp->irq);
- return res;
+ return -EAGAIN;
}
}
--
Ueimor
Will code drivers for food.
^ permalink raw reply related
* Re: [REGRESSION][PATCH v1] bpf jit: Let the arm jit handle negative memory references
From: Eric Dumazet @ 2012-04-07 3:41 UTC (permalink / raw)
To: Mircea Gherzan
Cc: Jan Seiffert, netdev, linux-kernel, David S. Miller, Russell King
In-Reply-To: <4F7F6DF9.5010902@gmail.com>
On Sat, 2012-04-07 at 00:28 +0200, Mircea Gherzan wrote:
> Hi,
>
> Am 06.04.2012 20:57, schrieb Jan Seiffert:
> > The arm jit has the same problem as the other two jits.
> > It only tests for negative absolute memory references, and if it sees
> > one bails out to let the interpreter handle the filter program.
> >
> > But this fails if the X register contains a negative memory reference
> > and is used for an indirect load.
>
> I don't think that there's any use case for negative values in X. In
> both the original BPF design and in the LSF interpreter, A and X are
> considered unsigned. The role of X is mainly to allow variable length
> headers (load the length -> unsigned).
>
> "Negative" K values are permitted for ancillary data loads based on the
> fact that we're not going to see 2GB packets any time soon.
>
You are wrong.
Please carefully read net/core/filter.c its all here :
void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size)
{
u8 *ptr = NULL;
if (k >= SKF_NET_OFF)
ptr = skb_network_header(skb) + k - SKF_NET_OFF;
else if (k >= SKF_LL_OFF)
ptr = skb_mac_header(skb) + k - SKF_LL_OFF;
if (ptr >= skb->head && ptr + size <= skb_tail_pointer(skb))
return ptr;
return NULL;
}
Then :
commit a998d4342337c82dacdc0897d30a9364de1576a1
Author: Jan Seiffert <kaffeemonster@googlemail.com>
Date: Fri Mar 30 05:24:05 2012 +0000
bpf jit: Let the x86 jit handle negative offsets
Now the helper function from filter.c for negative offsets is exported,
it can be used it in the jit to handle negative offsets.
First modify the asm load helper functions to handle:
- know positive offsets
- know negative offsets
- any offset
then the compiler can be modified to explicitly use these helper
when appropriate.
This fixes the case of a negative X register and allows to lift
the restriction that bpf programs with negative offsets can't
be jited.
Signed-of-by: Jan Seiffert <kaffeemonster@googlemail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
^ permalink raw reply
* [PATCH 2/2] phonet: Sort out initiailziation and cleanup code.
From: Eric W. Biederman @ 2012-04-07 1:35 UTC (permalink / raw)
To: David Miller
Cc: Eric Dumazet, Eric Van Hensbergen, Dave Jones, linux-kernel,
netdev, Sasha Levin, Remi Denis-Courmont
In-Reply-To: <m1aa2otjf4.fsf_-_@fess.ebiederm.org>
Recently an oops was reported in phonet if there was a failure during
network namespace creation.
[ 163.733755] ------------[ cut here ]------------
[ 163.734501] kernel BUG at include/net/netns/generic.h:45!
[ 163.734501] invalid opcode: 0000 [#1] PREEMPT SMP
[ 163.734501] CPU 2
[ 163.734501] Pid: 19145, comm: trinity Tainted: G W 3.4.0-rc1-next-20120405-sasha-dirty #57
[ 163.734501] RIP: 0010:[<ffffffff824d6062>] [<ffffffff824d6062>] phonet_pernet+0x182/0x1a0
[ 163.734501] RSP: 0018:ffff8800674d5ca8 EFLAGS: 00010246
[ 163.734501] RAX: 000000003fffffff RBX: 0000000000000000 RCX: ffff8800678c88d8
[ 163.734501] RDX: 00000000003f4000 RSI: ffff8800678c8910 RDI: 0000000000000282
[ 163.734501] RBP: ffff8800674d5cc8 R08: 0000000000000000 R09: 0000000000000000
[ 163.734501] R10: 0000000000000000 R11: 0000000000000000 R12: ffff880068bec920
[ 163.734501] R13: ffffffff836b90c0 R14: 0000000000000000 R15: 0000000000000000
[ 163.734501] FS: 00007f055e8de700(0000) GS:ffff88007d000000(0000) knlGS:0000000000000000
[ 163.734501] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[ 163.734501] CR2: 00007f055e6bb518 CR3: 0000000070c16000 CR4: 00000000000406e0
[ 163.734501] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 163.734501] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
[ 163.734501] Process trinity (pid: 19145, threadinfo ffff8800674d4000, task ffff8800678c8000)
[ 163.734501] Stack:
[ 163.734501] ffffffff824d5f00 ffffffff810e2ec1 ffff880067ae0000 00000000ffffffd4
[ 163.734501] ffff8800674d5cf8 ffffffff824d667a ffff880067ae0000 00000000ffffffd4
[ 163.734501] ffffffff836b90c0 0000000000000000 ffff8800674d5d18 ffffffff824d707d
[ 163.734501] Call Trace:
[ 163.734501] [<ffffffff824d5f00>] ? phonet_pernet+0x20/0x1a0
[ 163.734501] [<ffffffff810e2ec1>] ? get_parent_ip+0x11/0x50
[ 163.734501] [<ffffffff824d667a>] phonet_device_destroy+0x1a/0x100
[ 163.734501] [<ffffffff824d707d>] phonet_device_notify+0x3d/0x50
[ 163.734501] [<ffffffff810dd96e>] notifier_call_chain+0xee/0x130
[ 163.734501] [<ffffffff810dd9d1>] raw_notifier_call_chain+0x11/0x20
[ 163.734501] [<ffffffff821cce12>] call_netdevice_notifiers+0x52/0x60
[ 163.734501] [<ffffffff821cd235>] rollback_registered_many+0x185/0x270
[ 163.734501] [<ffffffff821cd334>] unregister_netdevice_many+0x14/0x60
[ 163.734501] [<ffffffff823123e3>] ipip_exit_net+0x1b3/0x1d0
[ 163.734501] [<ffffffff82312230>] ? ipip_rcv+0x420/0x420
[ 163.734501] [<ffffffff821c8515>] ops_exit_list+0x35/0x70
[ 163.734501] [<ffffffff821c911b>] setup_net+0xab/0xe0
[ 163.734501] [<ffffffff821c9416>] copy_net_ns+0x76/0x100
[ 163.734501] [<ffffffff810dc92b>] create_new_namespaces+0xfb/0x190
[ 163.734501] [<ffffffff810dca21>] unshare_nsproxy_namespaces+0x61/0x80
[ 163.734501] [<ffffffff810afd1f>] sys_unshare+0xff/0x290
[ 163.734501] [<ffffffff8187622e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
[ 163.734501] [<ffffffff82665539>] system_call_fastpath+0x16/0x1b
[ 163.734501] Code: e0 c3 fe 66 0f 1f 44 00 00 48 c7 c2 40 60 4d 82 be 01 00 00 00 48 c7 c7 80 d1 23 83 e8 48 2a c4 fe e8 73 06 c8 fe 48 85 db 75 0e <0f> 0b 0f 1f 40 00 eb fe 66 0f 1f 44 00 00 48 83 c4 10 48 89 d8
[ 163.734501] RIP [<ffffffff824d6062>] phonet_pernet+0x182/0x1a0
[ 163.734501] RSP <ffff8800674d5ca8>
[ 163.861289] ---[ end trace fb5615826c548066 ]---
After investigation it turns out there were two issues.
1) Phonet was not implementing network devices but was using register_pernet_device
instead of register_pernet_subsys.
This was allowing there to be cases when phonenet was not initialized and
the phonet net_generic was not set for a network namespace when network
device events were being reported on the netdevice_notifier for a network
namespace leading to the oops above.
2) phonet_exit_net was implementing a confusing and special case of handling all
network devices from going away that it was hard to see was correct, and would
only occur when the phonet module was removed.
Now that unregister_netdevice_notifier has been modified to synthesize unregistration
events for the network devices that are extant when called this confusing special
case in phonet_exit_net is no longer needed.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
net/phonet/pn_dev.c | 21 ++-------------------
1 files changed, 2 insertions(+), 19 deletions(-)
diff --git a/net/phonet/pn_dev.c b/net/phonet/pn_dev.c
index 9b9a85e..bf5cf69 100644
--- a/net/phonet/pn_dev.c
+++ b/net/phonet/pn_dev.c
@@ -331,23 +331,6 @@ static int __net_init phonet_init_net(struct net *net)
static void __net_exit phonet_exit_net(struct net *net)
{
- struct phonet_net *pnn = phonet_pernet(net);
- struct net_device *dev;
- unsigned i;
-
- rtnl_lock();
- for_each_netdev(net, dev)
- phonet_device_destroy(dev);
-
- for (i = 0; i < 64; i++) {
- dev = pnn->routes.table[i];
- if (dev) {
- rtm_phonet_notify(RTM_DELROUTE, dev, i);
- dev_put(dev);
- }
- }
- rtnl_unlock();
-
proc_net_remove(net, "phonet");
}
@@ -361,7 +344,7 @@ static struct pernet_operations phonet_net_ops = {
/* Initialize Phonet devices list */
int __init phonet_device_init(void)
{
- int err = register_pernet_device(&phonet_net_ops);
+ int err = register_pernet_subsys(&phonet_net_ops);
if (err)
return err;
@@ -377,7 +360,7 @@ void phonet_device_exit(void)
{
rtnl_unregister_all(PF_PHONET);
unregister_netdevice_notifier(&phonet_device_notifier);
- unregister_pernet_device(&phonet_net_ops);
+ unregister_pernet_subsys(&phonet_net_ops);
proc_net_remove(&init_net, "pnresource");
}
--
1.7.2.5
^ permalink raw reply related
* [PATCH 1/2] net: In unregister_netdevice_notifier unregister the netdevices.
From: Eric W. Biederman @ 2012-04-07 1:33 UTC (permalink / raw)
To: David Miller
Cc: Eric Dumazet, Eric Van Hensbergen, Dave Jones, linux-kernel,
netdev, Sasha Levin, Remi Denis-Courmont
In-Reply-To: <m1mx6otk85.fsf@fess.ebiederm.org>
>From a51fda849355b2ffb27cdb5d66fa1a96e7f47335 Mon Sep 17 00:00:00 2001
From: Eric W. Biederman <ebiederm@xmission.com>
Date: Sun, 25 Dec 2011 19:39:52 -0800
Subject:
We already synthesize events in register_netdevice_notifier and synthesizing
events in unregister_netdevice_notifier allows to us remove the need for
special case cleanup code.
This change should be safe as it adds no new cases for existing callers
of unregiser_netdevice_notifier to handle.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
net/core/dev.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 452db70..77c0a87 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1410,14 +1410,34 @@ EXPORT_SYMBOL(register_netdevice_notifier);
* register_netdevice_notifier(). The notifier is unlinked into the
* kernel structures and may then be reused. A negative errno code
* is returned on a failure.
+ *
+ * After unregistering unregister and down device events are synthesized
+ * for all devices on the device list to the removed notifier to remove
+ * the need for special case cleanup code.
*/
int unregister_netdevice_notifier(struct notifier_block *nb)
{
+ struct net_device *dev;
+ struct net *net;
int err;
rtnl_lock();
err = raw_notifier_chain_unregister(&netdev_chain, nb);
+ if (err)
+ goto unlock;
+
+ for_each_net(net) {
+ for_each_netdev(net, dev) {
+ if (dev->flags & IFF_UP) {
+ nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
+ nb->notifier_call(nb, NETDEV_DOWN, dev);
+ }
+ nb->notifier_call(nb, NETDEV_UNREGISTER, dev);
+ nb->notifier_call(nb, NETDEV_UNREGISTER_BATCH, dev);
+ }
+ }
+unlock:
rtnl_unlock();
return err;
}
--
1.7.2.5
^ permalink raw reply related
* Re: net: kernel BUG() in net/netns/generic.h:45
From: [PATCH 1/2], net:In, unregister_netdevice_notifier, unregister, the, netdevices. @ 2012-04-07 1:31 UTC (permalink / raw)
To: David Miller
Cc: Eric Dumazet, Eric Van Hensbergen, Dave Jones, linux-kernel,
netdev, Rémi Denis-Courmont, Sasha Levin
In-Reply-To: <m1mx6otk85.fsf@fess.ebiederm.org>
We already synthesize events in register_netdevice_notifier and synthesizing
events in unregister_netdevice_notifier allows to us remove the need for
special case cleanup code.
This change should be trivially safe as it adds no new cases for
existing callers of unregister_netdevice_notifier to handle.
Signed-off-by: Eric W. Biederman <ebiederm@xmission.com>
---
net/core/dev.c | 20 ++++++++++++++++++++
1 files changed, 20 insertions(+), 0 deletions(-)
diff --git a/net/core/dev.c b/net/core/dev.c
index 452db70..77c0a87 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -1410,14 +1410,34 @@ EXPORT_SYMBOL(register_netdevice_notifier);
* register_netdevice_notifier(). The notifier is unlinked into the
* kernel structures and may then be reused. A negative errno code
* is returned on a failure.
+ *
+ * After unregistering unregister and down device events are synthesized
+ * for all devices on the device list to the removed notifier to remove
+ * the need for special case cleanup code.
*/
int unregister_netdevice_notifier(struct notifier_block *nb)
{
+ struct net_device *dev;
+ struct net *net;
int err;
rtnl_lock();
err = raw_notifier_chain_unregister(&netdev_chain, nb);
+ if (err)
+ goto unlock;
+
+ for_each_net(net) {
+ for_each_netdev(net, dev) {
+ if (dev->flags & IFF_UP) {
+ nb->notifier_call(nb, NETDEV_GOING_DOWN, dev);
+ nb->notifier_call(nb, NETDEV_DOWN, dev);
+ }
+ nb->notifier_call(nb, NETDEV_UNREGISTER, dev);
+ nb->notifier_call(nb, NETDEV_UNREGISTER_BATCH, dev);
+ }
+ }
+unlock:
rtnl_unlock();
return err;
}
--
1.7.2.5
^ permalink raw reply related
* Re: net: kernel BUG() in net/netns/generic.h:45
From: Eric W. Biederman @ 2012-04-07 1:16 UTC (permalink / raw)
To: Sasha Levin
Cc: davem, Eric Dumazet, Eric Van Hensbergen, Dave Jones,
linux-kernel, netdev
In-Reply-To: <CA+1xoqdi4QOhRJvBA64pxiN3EphhThjPkf4eLRkYM8jxcJjh=w@mail.gmail.com>
Sasha Levin <levinsasha928@gmail.com> writes:
> On Fri, Apr 6, 2012 at 1:53 AM, Eric W. Biederman <ebiederm@xmission.com> wrote:
>> Sasha Levin <levinsasha928@gmail.com> writes:
>>
>>> Hi all,
>>>
>>> When an initialization of a network namespace in setup_net() fails, we
>>> try to undo everything by executing each of the exit callbacks of every
>>> namespace in the network.
>>>
>>> The problem is, it might be possible that the net_generic array wasn't
>>> initialized before we fail and try to undo everything. At that point,
>>> some of the networks assume that since we're already calling the exit
>>> callback, the net_generic structure is initialized and we hit the BUG()
>>> in net/netns/generic.h:45 .
>>>
>>> I'm not quite sure whether the right fix from the following three
>>> options is, and would be happy to figure it out before fixing it:
>>>
>>> 1. Don't assume net_generic was initialized in the exit callback, which
>>> is a bit problematic since we can't query that nicely anyway (a
>>> sub-option here would be adding an API to query whether the net_generic
>>> structure is initialized.
>>>
>>> 2. Remove the BUG(), switch it to a WARN() and let each subsystem
>>> handle the case of NULL on it's own. While it sounds a bit wrong, it's
>>> worth mentioning that that BUG() was initially added in an attempt to
>>> fix an issue in CAIF, which was fixed in a completely different way
>>> afterwards, so it's not strictly necessary here.
>>>
>>> 3. Only call the exit callback for subsystems we have called the init
>>> callback for.
>>
>> Your option 3 only calling the exit callbacks for subsystems we have
>> initialized should be what is implemented. Certainly it looks like we
>> are attempting to only call the exit callbacks for code whose init
>> callback has succeeded.
>>
>> What problem are you seeing?
>>
>> This smells suspiciously like a problem we had a little while ago caif
>> was registering as a pernet device instead of a pernet subsystem,
>> and because of that we had packets flying around after it had been
>> unregistered and was trying access it's net_generic data.
>
> It looks different from the caif problem a bit, here is a sample stacktrace:
>
> [ 163.733755] ------------[ cut here ]------------
> [ 163.734501] kernel BUG at include/net/netns/generic.h:45!
> [ 163.734501] invalid opcode: 0000 [#1] PREEMPT SMP
> [ 163.734501] CPU 2
> [ 163.734501] Pid: 19145, comm: trinity Tainted: G W
> 3.4.0-rc1-next-20120405-sasha-dirty #57
> [ 163.734501] RIP: 0010:[<ffffffff824d6062>] [<ffffffff824d6062>]
> phonet_pernet+0x182/0x1a0
> [ 163.734501] RSP: 0018:ffff8800674d5ca8 EFLAGS: 00010246
> [ 163.734501] RAX: 000000003fffffff RBX: 0000000000000000 RCX: ffff8800678c88d8
> [ 163.734501] RDX: 00000000003f4000 RSI: ffff8800678c8910 RDI: 0000000000000282
> [ 163.734501] RBP: ffff8800674d5cc8 R08: 0000000000000000 R09: 0000000000000000
> [ 163.734501] R10: 0000000000000000 R11: 0000000000000000 R12: ffff880068bec920
> [ 163.734501] R13: ffffffff836b90c0 R14: 0000000000000000 R15: 0000000000000000
> [ 163.734501] FS: 00007f055e8de700(0000) GS:ffff88007d000000(0000)
> knlGS:0000000000000000
> [ 163.734501] CS: 0010 DS: 0000 ES: 0000 CR0: 000000008005003b
> [ 163.734501] CR2: 00007f055e6bb518 CR3: 0000000070c16000 CR4: 00000000000406e0
> [ 163.734501] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> [ 163.734501] DR3: 0000000000000000 DR6: 00000000ffff0ff0 DR7: 0000000000000400
> [ 163.734501] Process trinity (pid: 19145, threadinfo
> ffff8800674d4000, task ffff8800678c8000)
> [ 163.734501] Stack:
> [ 163.734501] ffffffff824d5f00 ffffffff810e2ec1 ffff880067ae0000
> 00000000ffffffd4
> [ 163.734501] ffff8800674d5cf8 ffffffff824d667a ffff880067ae0000
> 00000000ffffffd4
> [ 163.734501] ffffffff836b90c0 0000000000000000 ffff8800674d5d18
> ffffffff824d707d
> [ 163.734501] Call Trace:
> [ 163.734501] [<ffffffff824d5f00>] ? phonet_pernet+0x20/0x1a0
> [ 163.734501] [<ffffffff810e2ec1>] ? get_parent_ip+0x11/0x50
> [ 163.734501] [<ffffffff824d667a>] phonet_device_destroy+0x1a/0x100
> [ 163.734501] [<ffffffff824d707d>] phonet_device_notify+0x3d/0x50
> [ 163.734501] [<ffffffff810dd96e>] notifier_call_chain+0xee/0x130
> [ 163.734501] [<ffffffff810dd9d1>] raw_notifier_call_chain+0x11/0x20
> [ 163.734501] [<ffffffff821cce12>] call_netdevice_notifiers+0x52/0x60
> [ 163.734501] [<ffffffff821cd235>] rollback_registered_many+0x185/0x270
> [ 163.734501] [<ffffffff821cd334>] unregister_netdevice_many+0x14/0x60
> [ 163.734501] [<ffffffff823123e3>] ipip_exit_net+0x1b3/0x1d0
> [ 163.734501] [<ffffffff82312230>] ? ipip_rcv+0x420/0x420
> [ 163.734501] [<ffffffff821c8515>] ops_exit_list+0x35/0x70
> [ 163.734501] [<ffffffff821c911b>] setup_net+0xab/0xe0
> [ 163.734501] [<ffffffff821c9416>] copy_net_ns+0x76/0x100
> [ 163.734501] [<ffffffff810dc92b>] create_new_namespaces+0xfb/0x190
> [ 163.734501] [<ffffffff810dca21>] unshare_nsproxy_namespaces+0x61/0x80
> [ 163.734501] [<ffffffff810afd1f>] sys_unshare+0xff/0x290
> [ 163.734501] [<ffffffff8187622e>] ? trace_hardirqs_on_thunk+0x3a/0x3f
> [ 163.734501] [<ffffffff82665539>] system_call_fastpath+0x16/0x1b
> [ 163.734501] Code: e0 c3 fe 66 0f 1f 44 00 00 48 c7 c2 40 60 4d 82
> be 01 00 00 00 48 c7 c7 80 d1 23 83 e8 48 2a c4 fe e8 73 06 c8 fe 48
> 85 db 75 0e <0f> 0b 0f 1f 40 00 eb fe 66 0f 1f 44 00 00 48 83 c4 10 48
> 89 d8
> [ 163.734501] RIP [<ffffffff824d6062>] phonet_pernet+0x182/0x1a0
> [ 163.734501] RSP <ffff8800674d5ca8>
> [ 163.861289] ---[ end trace fb5615826c548066 ]---
>
> It would appear that there's at least a one-off problem with the
> reverse iteration.
I don't see anything pointing to reverse iteration being the problem,
or even the call graph order.
What I see is phonet registering a netdevice notifier.
Then I see phonet register pernet_device operations.
Then I see phonet responding to all network devices no matter what
the network namespace is, and looking in the phonet net_generic data
for them.
Looking phonet does not implement any network devices phonet just has
per network device state. Which means phonet should be using
register_pernet_subsys and this roughly is the same issue we saw with
caif. Confusion of when you should use register_pernet_subsys and
register_pernet_device.
Issues that I see.
- There is a bunch of weird and stuff going on in phonet_net_exit
to handle the module unregister case where we don't synthesize
network device removal events. ( A generic bug we can fix).
- phonet should be using register_pernet_subsys instead of
register_pernet_device.
Patches to follow in a minute. Is there any chance you can reproduce
this so you can verify the problems don't continue to reproduce?
Eric
^ permalink raw reply
* Re: Bug#659499: bash fails to properly read /proc files
From: Ben Hutchings @ 2012-04-07 1:01 UTC (permalink / raw)
To: Mihai Maruseac
Cc: Michael Stone, 659499, debian-kernel, Cyril Brulebois,
Jean-Michel Vourgère, netdev
In-Reply-To: <1330215119.8460.18.camel@deadeye>
[-- Attachment #1.1: Type: text/plain, Size: 1262 bytes --]
On Sun, 2012-02-26 at 00:11 +0000, Ben Hutchings wrote:
> On Sun, 2012-02-19 at 23:14 +0100, Cyril Brulebois wrote:
> > Hi kernel folks,
> >
> > here's a tiny analysis I tried to perform on bash's having issues with
> > reading /proc files, which I think is related to seeking in those files.
> > I can't play much with other kernel versions right now though. My tests
> > were performed with squeeze's bpo kernel: 3.2.0-0.bpo.1-amd64 (Debian
> > 3.2.4-1~bpo60+1).
>
> The specific problem with seeking in /proc/net/dev appears to be caused
> by this change:
>
> commit f04565ddf52e401880f8ba51de0dff8ba51c99fd
> Author: Mihai Maruseac <mihai.maruseac@gmail.com>
> Date: Thu Oct 20 20:45:10 2011 +0000
>
> dev: use name hash for dev_seq_ops
[...]
This is supposed to be fixed by:
commit 2def16ae6b0c77571200f18ba4be049b03d75579
Author: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon Apr 2 22:33:02 2012 +0000
net: fix /proc/net/dev regression
which will be applied some time soon. I'm attaching the patch in case
anyone would like to test it (see
<http://kernel-handbook.alioth.debian.org/ch-common-tasks.html#s-common-official>).
Ben.
--
Ben Hutchings
Larkinson's Law: All laws are basically false.
[-- Attachment #1.2: net-fix-proc-net-dev-regression.patch --]
[-- Type: text/x-patch, Size: 5983 bytes --]
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 2 Apr 2012 22:33:02 +0000
Subject: [PATCH] net: fix /proc/net/dev regression
commit 2def16ae6b0c77571200f18ba4be049b03d75579 upstream.
Commit f04565ddf52 (dev: use name hash for dev_seq_ops) added a second
regression, as some devices are missing from /proc/net/dev if many
devices are defined.
When seq_file buffer is filled, the last ->next/show() method is
canceled (pos value is reverted to value prior ->next() call)
Problem is after above commit, we dont restart the lookup at right
position in ->start() method.
Fix this by removing the internal 'pos' pointer added in commit, since
we need to use the 'loff_t *pos' provided by seq_file layer.
This also reverts commit 5cac98dd0 (net: Fix corruption
in /proc/*/net/dev_mcast), since its not needed anymore.
Reported-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
Cc: Mihai Maruseac <mmaruseac@ixiacom.com>
Tested-by: Ben Greear <greearb@candelatech.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
---
include/linux/netdevice.h | 2 --
net/core/dev.c | 58 ++++++++++-----------------------------------
net/core/dev_addr_lists.c | 3 ++-
3 files changed, 15 insertions(+), 48 deletions(-)
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 1f77540..5cbaa20 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -2604,8 +2604,6 @@ extern void net_disable_timestamp(void);
extern void *dev_seq_start(struct seq_file *seq, loff_t *pos);
extern void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos);
extern void dev_seq_stop(struct seq_file *seq, void *v);
-extern int dev_seq_open_ops(struct inode *inode, struct file *file,
- const struct seq_operations *ops);
#endif
extern int netdev_class_create_file(struct class_attribute *class_attr);
diff --git a/net/core/dev.c b/net/core/dev.c
index 6c7dc9d..c25d453 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -4028,54 +4028,41 @@ static int dev_ifconf(struct net *net, char __user *arg)
#ifdef CONFIG_PROC_FS
-#define BUCKET_SPACE (32 - NETDEV_HASHBITS)
-
-struct dev_iter_state {
- struct seq_net_private p;
- unsigned int pos; /* bucket << BUCKET_SPACE + offset */
-};
+#define BUCKET_SPACE (32 - NETDEV_HASHBITS - 1)
#define get_bucket(x) ((x) >> BUCKET_SPACE)
#define get_offset(x) ((x) & ((1 << BUCKET_SPACE) - 1))
#define set_bucket_offset(b, o) ((b) << BUCKET_SPACE | (o))
-static inline struct net_device *dev_from_same_bucket(struct seq_file *seq)
+static inline struct net_device *dev_from_same_bucket(struct seq_file *seq, loff_t *pos)
{
- struct dev_iter_state *state = seq->private;
struct net *net = seq_file_net(seq);
struct net_device *dev;
struct hlist_node *p;
struct hlist_head *h;
- unsigned int count, bucket, offset;
+ unsigned int count = 0, offset = get_offset(*pos);
- bucket = get_bucket(state->pos);
- offset = get_offset(state->pos);
- h = &net->dev_name_head[bucket];
- count = 0;
+ h = &net->dev_name_head[get_bucket(*pos)];
hlist_for_each_entry_rcu(dev, p, h, name_hlist) {
- if (count++ == offset) {
- state->pos = set_bucket_offset(bucket, count);
+ if (++count == offset)
return dev;
- }
}
return NULL;
}
-static inline struct net_device *dev_from_new_bucket(struct seq_file *seq)
+static inline struct net_device *dev_from_bucket(struct seq_file *seq, loff_t *pos)
{
- struct dev_iter_state *state = seq->private;
struct net_device *dev;
unsigned int bucket;
- bucket = get_bucket(state->pos);
do {
- dev = dev_from_same_bucket(seq);
+ dev = dev_from_same_bucket(seq, pos);
if (dev)
return dev;
- bucket++;
- state->pos = set_bucket_offset(bucket, 0);
+ bucket = get_bucket(*pos) + 1;
+ *pos = set_bucket_offset(bucket, 1);
} while (bucket < NETDEV_HASHENTRIES);
return NULL;
@@ -4088,33 +4075,20 @@ static inline struct net_device *dev_from_new_bucket(struct seq_file *seq)
void *dev_seq_start(struct seq_file *seq, loff_t *pos)
__acquires(RCU)
{
- struct dev_iter_state *state = seq->private;
-
rcu_read_lock();
if (!*pos)
return SEQ_START_TOKEN;
- /* check for end of the hash */
- if (state->pos == 0 && *pos > 1)
+ if (get_bucket(*pos) >= NETDEV_HASHENTRIES)
return NULL;
- return dev_from_new_bucket(seq);
+ return dev_from_bucket(seq, pos);
}
void *dev_seq_next(struct seq_file *seq, void *v, loff_t *pos)
{
- struct net_device *dev;
-
++*pos;
-
- if (v == SEQ_START_TOKEN)
- return dev_from_new_bucket(seq);
-
- dev = dev_from_same_bucket(seq);
- if (dev)
- return dev;
-
- return dev_from_new_bucket(seq);
+ return dev_from_bucket(seq, pos);
}
void dev_seq_stop(struct seq_file *seq, void *v)
@@ -4213,13 +4187,7 @@ static const struct seq_operations dev_seq_ops = {
static int dev_seq_open(struct inode *inode, struct file *file)
{
return seq_open_net(inode, file, &dev_seq_ops,
- sizeof(struct dev_iter_state));
-}
-
-int dev_seq_open_ops(struct inode *inode, struct file *file,
- const struct seq_operations *ops)
-{
- return seq_open_net(inode, file, ops, sizeof(struct dev_iter_state));
+ sizeof(struct seq_net_private));
}
static const struct file_operations dev_seq_fops = {
diff --git a/net/core/dev_addr_lists.c b/net/core/dev_addr_lists.c
index 29c07fe..626698f 100644
--- a/net/core/dev_addr_lists.c
+++ b/net/core/dev_addr_lists.c
@@ -696,7 +696,8 @@ static const struct seq_operations dev_mc_seq_ops = {
static int dev_mc_seq_open(struct inode *inode, struct file *file)
{
- return dev_seq_open_ops(inode, file, &dev_mc_seq_ops);
+ return seq_open_net(inode, file, &dev_mc_seq_ops,
+ sizeof(struct seq_net_private));
}
static const struct file_operations dev_mc_seq_fops = {
--
1.7.9.5
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 828 bytes --]
^ permalink raw reply related
* Re: ipv6: tunnel: hang when destroying ipv6 tunnel
From: Tetsuo Handa @ 2012-04-07 0:06 UTC (permalink / raw)
To: garlick
Cc: levinsasha928, ericvh, oleg, eric.dumazet, davem, kuznet, jmorris,
yoshfuji, kaber, netdev, linux-kernel, davej
In-Reply-To: <20120406180905.GA10835@llnl.gov>
Jim Garlick wrote:
> > I think this loop is bad with regard to response to SIGKILL.
> > If wait_event_interruptible() was interrupted by SIGKILL, it will
> > spin until req->status >= REQ_STATUS_RCVD becomes true.
> > Rather,
> >
> > if ((c->status == Connected) && (type == P9_TFLUSH))
> > err = wait_event_killable(*req->wq,
> > req->status >= REQ_STATUS_RCVD);
> > else
> > err = wait_event_interruptible(*req->wq,
> > req->status >= REQ_STATUS_RCVD);
> >
> > would be safer.
>
> Does that work? What prevents p9_client_rpc() from recursing via
> p9_client_flush() on receipt of SIGKILL?
Sorry, I'm not a 9p user and I can't test whether that works or not.
But at least, continuing the loop even after SIGKILL is not good.
If you have to wait for req->status >= REQ_STATUS_RCVD becomes true, can you
use a kernel thread that waits req->status >= REQ_STATUS_RCVD to become true
and delegate the job of notifying the server from a userspace task to the
kernel thread?
> Yes but in the unlikely event that this happens, the effect is a small
> memory leak for the duration of the mount. On the other hand if the
> fid is destroyed without successfully informing the server, then
> subsequent operations that involve new file references will fail
> when that fid number is reused, and the mount becomes unusable.
I don't know whether Sasha's problem is caused by this patch or not.
But p9_client_clunk() is called from many functions in fs/9p/ directory.
They are assuming that p9_client_clunk() will call p9_fid_destroy() but
this patch is breaking that assumption. I think this is the cause of hang which
Sasha is experiencing because Sasha's trace shows that call_usermodehelper() is
blocked by functions in fs/9p/ directory. Seems inconsistency state problem.
^ permalink raw reply
* Re: [REGRESSION][PATCH v1] bpf jit: Let the arm jit handle negative memory references
From: Jan Seiffert @ 2012-04-06 23:30 UTC (permalink / raw)
To: Mircea Gherzan
Cc: netdev, linux-kernel, Eric Dumazet, David S. Miller, Russell King
In-Reply-To: <4F7F6DF9.5010902@gmail.com>
Mircea Gherzan schrieb:
> Hi,
>
Hi
> Am 06.04.2012 20:57, schrieb Jan Seiffert:
>> The arm jit has the same problem as the other two jits.
>> It only tests for negative absolute memory references, and if it sees
>> one bails out to let the interpreter handle the filter program.
>>
>> But this fails if the X register contains a negative memory reference
>> and is used for an indirect load.
>
> I don't think that there's any use case for negative values in X.
<mode temper="elevated" reason="had that discussion already">
There is.
Say you have a bpf filter on an UDP socket. You want to filter bogus
Source addresses in kernel to prevent the context switch and copying.
Since ipv6 has a v4 mapped space, you have to make the same tests for
the v6-mapped space as for v4, so you can use one program for v4 & v6
to begin with.
You can solve it without a negative offset in X, it's is just very
helpful. Still you have negative offsets, which means no jit.
struct bpf_insn udp_filter[] = {
/* 0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(12)),
/* 1 */ BPF_STMT(BPF_LD|BPF_B|BPF_ABS, -1048576+(0)),
/* 2 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xf0),
/* 3 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x40, 23 - 4, 0),
/* 4 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x60, 5 - 5, 41 - 5),
/* 5 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(8)),
/* 6 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 13 - 7, 0),
/* 7 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010DB8, 41 - 8, 0),
/* 8 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010002, 19 - 9, 0),
/* 9 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xfffffff0),
/* 10 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x20010010, 41 - 11, 0),
/* 11 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xff000000),
/* 12 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xff000000, 41 - 13, 39 - 13),
/* 13 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(12)),
/* 14 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 0, 39 - 15),
/* 15 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(16)),
/* 16 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xffff, 22 - 17, 0),
/* 17 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0x0064FF9B, 22 - 18, 0),
/* 18 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 41 - 19, 39 - 19),
/* 19 */ BPF_STMT(BPF_LD|BPF_W|BPF_ABS, -1048576+(12)),
/* 20 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xffff0000),
/* 21 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 41 - 22, 39 - 22),
/* 22 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(20)),
/* 23 */ BPF_STMT(BPF_LD|BPF_W|BPF_IND, 0),
/* 24 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xffffffff, 41 - 25, 0),
/* 25 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xffffff00),
/* 26 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0000000, 41 - 27, 0),
/* 27 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0000200, 41 - 28, 0),
/* 28 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC6336400, 41 - 29, 0),
/* 29 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xCB007100, 41 - 30, 0),
/* 30 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC0586300, 41 - 31, 0),
/* 31 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xfffe0000),
/* 32 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xC6120000, 41 - 33, 0),
/* 33 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xff000000),
/* 34 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0, 41 - 35, 0),
/* 35 */ BPF_STMT(BPF_ALU|BPF_AND|BPF_K, 0xf0000000),
/* 36 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xE0000000, 41 - 37, 0),
/* 37 */ BPF_JUMP(BPF_JMP|BPF_JEQ|BPF_K, 0xF0000000, 41 - 38, 0),
/* 38 */ BPF_JUMP(BPF_JMP|BPF_JA, 39 - 39, 0, 0),
/* 39 */ BPF_STMT(BPF_LD|BPF_W|BPF_LEN, 0),
/* 40 */ BPF_STMT(BPF_RET|BPF_A, 0),
/* 41 */ BPF_STMT(BPF_RET|BPF_K, 0),
};
I already had that discussion with Eric, he said it is a valid use case.
http://marc.info/?l=linux-netdev&m=133296726719053&w=2
The patch for x86 is already in, ppc is being reviewed, i only heard of the
arm jit to late, or you would have been in the loop from the beginning.
> In both the original BPF design and in the LSF interpreter, A and X are
> considered unsigned.
Then use the following test program (filter not usefull, just to show the problem):
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <pcap-bpf.h>
#define die(x) do {perror(x); return 1;} while (0)
struct bpf_insn udp_filter[] = {
/* 0 */ BPF_STMT(BPF_LDX|BPF_W|BPF_IMM, -1048576+(0)), /* leax net[0] */
/* 1 */ BPF_STMT(BPF_LD|BPF_B|BPF_IND, 0), /* ldb [x+0] */
/* 2 */ BPF_STMT(BPF_RET|BPF_A, 0), /* ret a */
};
int main(int argc, char *argv[])
{
char buf[512];
struct sockaddr_in addr;
struct bpf_program prg;
socklen_t addr_s;
ssize_t res;
int fd;
addr.sin_family = AF_INET;
addr.sin_port = htons(5000);
addr.sin_addr.s_addr = 0;
addr_s = sizeof(addr);
prg.bf_len = sizeof(udp_filter)/sizeof(udp_filter[0]);
prg.bf_insns = udp_filter;
if(-1 == (fd = socket(AF_INET, SOCK_DGRAM, 0)))
die("socket");
if(-1 == bind(fd, (struct sockaddr *)&addr, sizeof(addr)))
die("bind");
if(-1 == setsockopt(fd, SOL_SOCKET, SO_ATTACH_FILTER, &prg, sizeof(prg)))
die("setsockopt");
res = recvfrom(fd, buf, sizeof(buf), 0, (struct sockaddr *)&addr, &addr_s);
if(res != -1)
printf("packet received: %zi bytes\n", res);
else
die("recvfrom");
return 0;
}
when used with the bpf jit disabled works:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1: packet received: 6 bytes
When the bpf jit gets enabled (echo 100 >
/proc/sys/net/core/bpf_jit_enable) the same program stops working:
console 1 $ ./bpf
console 2 $ echo "hello" | nc -u localhost 5000
console 1:
This is a regression in the interface in my book.
The interpreter handles this perfectly fine.
> The role of X is mainly to allow variable length
> headers (load the length -> unsigned).
>
> "Negative" K values are permitted for ancillary data loads based on the
> fact that we're not going to see 2GB packets any time soon.
>
Exactly, and how do i handle both at the same time? (The ancillary space
contains not only cpu_id and such stuff, but the link layer header and the network
space, which is important when you use LSF on arbitrary sockets, not just raw
sockets, maybe the new syscall filter thingy also wants a new ancillary space)
> Mircea
>
Have some nice holidays :-)
Greetings
Jan
^ permalink raw reply
* Re: [REGRESSION][PATCH v1] bpf jit: Let the arm jit handle negative memory references
From: Mircea Gherzan @ 2012-04-06 22:28 UTC (permalink / raw)
To: Jan Seiffert
Cc: netdev, linux-kernel, Eric Dumazet, David S. Miller, Russell King
In-Reply-To: <4F7F3C7D.1040007@googlemail.com>
Hi,
Am 06.04.2012 20:57, schrieb Jan Seiffert:
> The arm jit has the same problem as the other two jits.
> It only tests for negative absolute memory references, and if it sees
> one bails out to let the interpreter handle the filter program.
>
> But this fails if the X register contains a negative memory reference
> and is used for an indirect load.
I don't think that there's any use case for negative values in X. In
both the original BPF design and in the LSF interpreter, A and X are
considered unsigned. The role of X is mainly to allow variable length
headers (load the length -> unsigned).
"Negative" K values are permitted for ancillary data loads based on the
fact that we're not going to see 2GB packets any time soon.
Mircea
> This is only caught at runtime, where the filter will always drop
> the packet.
> Filter which work fine with the interpreter do not work with the jit.
>
> So this patch tries to fix it for the arm jit.
>
> First we add the prototype of bpf_internal_load_pointer_neg_helper to
> filter.h, since the arm jit has no assembler component, instead it has
> to be used from C.
>
> Then the helper functions are prepared to either handle known positive
> offsets, known negative offsets, or any offset and test at runtime.
>
> Finally the compiler is modified to emit calls to the right function,
> depending if either the offset is known, or not.
>
> This fixes the case of a negative X register and allows to lift
> the restriction that bpf programs with negative offsets can't
> be jited.
>
> Signed-off-by: Jan Seiffert <kaffeemonster@googlemail.com>
> ---
> The arm jit structure is a little bit different then the other jits, esp.
> it does not use assembler load helper. So i had to put the prototype for
> bpf_internal_load_pointer_neg_helper somewhere.
>
> This is a v1 to keep the ball rolling.
>
> Testing would also be cool, -ENOHARDWARE.
>
> arch/arm/net/bpf_jit_32.c | 149 ++++++++++++++++++++++++++++++++-------------
> include/linux/filter.h | 6 ++
> net/core/filter.c | 4 +
> 3 files changed, 117 insertions(+), 42 deletions(-)
>
> diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
> index 62135849..19d60af 100644
> --- a/arch/arm/net/bpf_jit_32.c
> +++ b/arch/arm/net/bpf_jit_32.c
> @@ -18,6 +18,7 @@
> #include <linux/slab.h>
> #include <asm/cacheflush.h>
> #include <asm/hwcap.h>
> +#include <asm/unaligned.h>
>
> #include "bpf_jit_32.h"
>
> @@ -71,7 +72,7 @@ struct jit_ctx {
>
> int bpf_jit_enable __read_mostly;
>
> -static u64 jit_get_skb_b(struct sk_buff *skb, unsigned offset)
> +static u64 jit_get_skb_b_pos(struct sk_buff *skb, unsigned offset)
> {
> u8 ret;
> int err;
> @@ -81,7 +82,7 @@ static u64 jit_get_skb_b(struct sk_buff *skb, unsigned offset)
> return (u64)err << 32 | ret;
> }
>
> -static u64 jit_get_skb_h(struct sk_buff *skb, unsigned offset)
> +static u64 jit_get_skb_h_pos(struct sk_buff *skb, unsigned offset)
> {
> u16 ret;
> int err;
> @@ -91,7 +92,7 @@ static u64 jit_get_skb_h(struct sk_buff *skb, unsigned offset)
> return (u64)err << 32 | ntohs(ret);
> }
>
> -static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset)
> +static u64 jit_get_skb_w_pos(struct sk_buff *skb, unsigned offset)
> {
> u32 ret;
> int err;
> @@ -101,6 +102,60 @@ static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset)
> return (u64)err << 32 | ntohl(ret);
> }
>
> +static u64 jit_get_skb_b_neg(struct sk_buff *skb, unsigned offset)
> +{
> + u8 *ptr;
> +
> + ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 1);
> + if (!ptr)
> + return (u64)1 << 32;
> + return *ptr;
> +}
> +
> +static u64 jit_get_skb_h_neg(struct sk_buff *skb, unsigned offset)
> +{
> + u16 *ptr;
> +
> + ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 2);
> + if (!ptr)
> + return (u64)1 << 32;
> + return get_unaligned_be16(ptr);
> +}
> +
> +static u64 jit_get_skb_w_neg(struct sk_buff *skb, unsigned offset)
> +{
> + u32 *ptr;
> +
> + ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 4);
> + if (!ptr)
> + return (u64)1 << 32;
> + return get_unaligned_be32(ptr);
> +}
> +
> +static u64 jit_get_skb_b_any(struct sk_buff *skb, unsigned offset)
> +{
> + if ((int)offset >= 0)
> + return jit_get_skb_b_pos(skb, offset);
> + else
> + return jit_get_skb_b_neg(skb, offset);
> +}
> +
> +static u64 jit_get_skb_h_any(struct sk_buff *skb, unsigned offset)
> +{
> + if ((int)offset >= 0)
> + return jit_get_skb_h_pos(skb, offset);
> + else
> + return jit_get_skb_h_neg(skb, offset);
> +}
> +
> +static u64 jit_get_skb_w_any(struct sk_buff *skb, unsigned offset)
> +{
> + if ((int)offset >= 0)
> + return jit_get_skb_w_pos(skb, offset);
> + else
> + return jit_get_skb_w_neg(skb, offset);
> +}
> +
> /*
> * Wrapper that handles both OABI and EABI and assures Thumb2 interworking
> * (where the assembly routines like __aeabi_uidiv could cause problems).
> @@ -458,7 +513,10 @@ static inline void update_on_xread(struct jit_ctx *ctx)
>
> static int build_body(struct jit_ctx *ctx)
> {
> - void *load_func[] = {jit_get_skb_b, jit_get_skb_h, jit_get_skb_w};
> + void *load_func_any[] = {jit_get_skb_b_any, jit_get_skb_h_any, jit_get_skb_w_any};
> + void *load_func_pos[] = {jit_get_skb_b_pos, jit_get_skb_h_pos, jit_get_skb_w_pos};
> + void *load_func_neg[] = {jit_get_skb_b_neg, jit_get_skb_h_neg, jit_get_skb_w_neg};
> + void **load_func;
> const struct sk_filter *prog = ctx->skf;
> const struct sock_filter *inst;
> unsigned i, load_order, off, condt;
> @@ -498,36 +556,38 @@ static int build_body(struct jit_ctx *ctx)
> case BPF_S_LD_B_ABS:
> load_order = 0;
> load:
> - /* the interpreter will deal with the negative K */
> - if ((int)k < 0)
> - return -ENOTSUPP;
> emit_mov_i(r_off, k, ctx);
> -load_common:
> - ctx->seen |= SEEN_DATA | SEEN_CALL;
> -
> - if (load_order > 0) {
> - emit(ARM_SUB_I(r_scratch, r_skb_hl,
> - 1 << load_order), ctx);
> - emit(ARM_CMP_R(r_scratch, r_off), ctx);
> - condt = ARM_COND_HS;
> - } else {
> - emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
> - condt = ARM_COND_HI;
> - }
>
> - _emit(condt, ARM_ADD_R(r_scratch, r_off, r_skb_data),
> - ctx);
> -
> - if (load_order == 0)
> - _emit(condt, ARM_LDRB_I(r_A, r_scratch, 0),
> + /* deal with negative K */
> + if (k >= 0) {
> + load_func = load_func_pos;
> + if (load_order > 0) {
> + emit(ARM_SUB_I(r_scratch, r_skb_hl,
> + 1 << load_order), ctx);
> + emit(ARM_CMP_R(r_scratch, r_off), ctx);
> + condt = ARM_COND_HS;
> + } else {
> + emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
> + condt = ARM_COND_HI;
> + }
> +
> + _emit(condt, ARM_ADD_R(r_scratch, r_off, r_skb_data),
> ctx);
> - else if (load_order == 1)
> - emit_load_be16(condt, r_A, r_scratch, ctx);
> - else if (load_order == 2)
> - emit_load_be32(condt, r_A, r_scratch, ctx);
>
> - _emit(condt, ARM_B(b_imm(i + 1, ctx)), ctx);
> + if (load_order == 0)
> + _emit(condt, ARM_LDRB_I(r_A, r_scratch, 0),
> + ctx);
> + else if (load_order == 1)
> + emit_load_be16(condt, r_A, r_scratch, ctx);
> + else if (load_order == 2)
> + emit_load_be32(condt, r_A, r_scratch, ctx);
>
> + _emit(condt, ARM_B(b_imm(i + 1, ctx)), ctx);
> + } else {
> + load_func = load_func_neg;
> + }
> +load_common:
> + ctx->seen |= SEEN_DATA | SEEN_CALL;
> /* the slowpath */
> emit_mov_i(ARM_R3, (u32)load_func[load_order], ctx);
> emit(ARM_MOV_R(ARM_R0, r_skb), ctx);
> @@ -547,7 +607,9 @@ load_common:
> case BPF_S_LD_B_IND:
> load_order = 0;
> load_ind:
> + load_func = load_func_any;
> OP_IMM3(ARM_ADD, r_off, r_X, k, ctx);
> + load_func = load_func_any;
> goto load_common;
> case BPF_S_LDX_IMM:
> ctx->seen |= SEEN_X;
> @@ -565,25 +627,28 @@ load_ind:
> case BPF_S_LDX_B_MSH:
> /* x = ((*(frame + k)) & 0xf) << 2; */
> ctx->seen |= SEEN_X | SEEN_DATA | SEEN_CALL;
> - /* the interpreter should deal with the negative K */
> - if (k < 0)
> - return -1;
> /* offset in r1: we might have to take the slow path */
> emit_mov_i(r_off, k, ctx);
> - emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
> + /* deal with negative K */
> + if (k >= 0) {
> + load_func = load_func_pos;
> + emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
>
> - /* load in r0: common with the slowpath */
> - _emit(ARM_COND_HI, ARM_LDRB_R(ARM_R0, r_skb_data,
> - ARM_R1), ctx);
> - /*
> - * emit_mov_i() might generate one or two instructions,
> - * the same holds for emit_blx_r()
> - */
> - _emit(ARM_COND_HI, ARM_B(b_imm(i + 1, ctx) - 2), ctx);
> + /* load in r0: common with the slowpath */
> + _emit(ARM_COND_HI, ARM_LDRB_R(ARM_R0, r_skb_data,
> + ARM_R1), ctx);
> + /*
> + * emit_mov_i() might generate one or two instructions,
> + * the same holds for emit_blx_r()
> + */
> + _emit(ARM_COND_HI, ARM_B(b_imm(i + 1, ctx) - 2), ctx);
> + } else {
> + load_func = load_func_neg;
> + }
>
> emit(ARM_MOV_R(ARM_R0, r_skb), ctx);
> /* r_off is r1 */
> - emit_mov_i(ARM_R3, (u32)jit_get_skb_b, ctx);
> + emit_mov_i(ARM_R3, (u32)load_func[0], ctx);
> emit_blx_r(ARM_R3, ctx);
> /* check the return value of skb_copy_bits */
> emit(ARM_CMP_I(ARM_R1, 0), ctx);
> diff --git a/include/linux/filter.h b/include/linux/filter.h
> index 8eeb205..78cd56d 100644
> --- a/include/linux/filter.h
> +++ b/include/linux/filter.h
> @@ -161,6 +161,12 @@ extern int sk_chk_filter(struct sock_filter *filter, unsigned int flen);
> extern void bpf_jit_compile(struct sk_filter *fp);
> extern void bpf_jit_free(struct sk_filter *fp);
> #define SK_RUN_FILTER(FILTER, SKB) (*FILTER->bpf_func)(SKB, FILTER->insns)
> +/*
> + * Only Exported for the bpf jit load helper.
> + * Do not call from anywhere else!
> + */
> +extern void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb,
> + int k, unsigned int size);
> #else
> static inline void bpf_jit_compile(struct sk_filter *fp)
> {
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 6f755cc..9cbaecb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -42,6 +42,10 @@
> /* No hurry in this branch
> *
> * Exported for the bpf jit load helper.
> + *
> + * CAUTION ! :
> + * If its prototype is ever changed, check arch/{*}/net/{*}.S files,
> + * since it is called from BPF assembly code.
> */
> void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size)
> {
>
--
Mircea
^ permalink raw reply
* Re: [PATCH net-next V2] tcp: Fix bug when gap in rcv sequence is filled
From: Neal Cardwell @ 2012-04-06 22:17 UTC (permalink / raw)
To: Vijay Subramanian; +Cc: chetan loke, netdev, davem, ilpo.jarvinen
In-Reply-To: <CAGK4HS-FZ038=Yik4DEQ26y4-9Zx9Gp80CU5_JS0U-anxjh9Wg@mail.gmail.com>
On Fri, Apr 6, 2012 at 10:36 AM, Vijay Subramanian
<subramanian.vijay@gmail.com> wrote:
> As per RFC2581 and the newer RFC5681, "the receiver SHOULD send an immediate ACK
> when it receives a data segment that fills in all or part of a gap in the
> sequence space." When TCP receiver gets the next in-sequence packet, we move
> data from ofo queue to receive queue. At this point, we should send an immediate
> ack by entering quickack mode. In the current code, instead of entering
> quickack mode upon requeing packets from ofo queue to receive_queue, we enter
> quickack mode only when ofo queue becomes empty after requeuing. This ignores
> the possibility that there may be further packets left in ofo queue. This patch
> fixes this behavior and enters quickack mode whenever packets are moved from ofo
> queue to receive queue.
If you have some packet traces documenting that there is actually a
bug here, can you please share them?
AFAICT the code correctly generates immediate ACKs in these cases by:
(a) having __tcp_ack_snd_check() call tcp_send_ack() to send an
immediate ack if out_of_order_queue is non-empty.
(b) having tcp_data_queue set icsk_ack.pingpong to 0 when
out_of_order_queue transitions from non-empty to empty
My testing also shows that Linux TCP correctly generates an immediate
ACK when we move data from the out_of_order_queue to the
sk_receive_queue but the out_of_order_queue is still non-empty. Here
is a condensed packet trace showing the behavior I see in tests (with
a trace taken on machine A):
.798 B>A . 50001:60001(10000) ack 1001 win 20000
.838 A>B . ack 60001 win 10000
.848 B>A . 61001:62001(1000) ack 1001 win 20000
.848 A>B . ack 60001 win 10000 <nop,nop,sack 1 {61001:62001}>
.858 B>A . 63001:64001(1000) ack 1001 win 20000
.858 A>B . ack 60001 win 10000 <nop,nop,sack 2 {63001:64001}{61001:62001}>
.868 B>A . 60001:61001(1000) ack 1001 win 20000
.868 A>B . ack 62001 win 10000 <nop,nop,sack 1 {63001:64001}>
neal
^ permalink raw reply
* [REGRESSION][PATCH v2] bpf jit: Let the arm jit handle negative memory references
From: Jan Seiffert @ 2012-04-06 21:48 UTC (permalink / raw)
To: netdev
Cc: linux-kernel, Eric Dumazet, David S. Miller, Mircea Gherzan,
Russell King
In-Reply-To: <4F7F3C7D.1040007@googlemail.com>
The arm jit has the same problem as the other two jits.
It only tests for negative absolute memory references, and if it sees
one bails out to let the interpreter handle the filter program.
But this fails if the X register contains a negative memory reference
and is used for an indirect load.
This is only caught at runtime, where the filter will always drop
the packet.
Filter which work fine with the interpreter do not work with the jit.
So this patch tries to fix it for the arm jit.
First we add the prototype of bpf_internal_load_pointer_neg_helper to
filter.h, since the arm jit has no assembler component, instead it has
to be used from C.
Then the helper functions are prepared to either handle known positive
offsets, known negative offsets, or any offset and test at runtime.
Finally the compiler is modified to emit calls to the right function,
depending if either the offset is known, or not.
This fixes the case of a negative X register and allows to lift
the restriction that bpf programs with negative offsets can't
be jited.
Signed-off-by: Jan Seiffert <kaffeemonster@googlemail.com>
---
Sorry for sending a v2 so fast, but i immidiatly found an major error...
v1 -> v2:
* when checking if K is greater then 0 make sure to compare signed
remarks from v1 still apply.
arch/arm/net/bpf_jit_32.c | 149 ++++++++++++++++++++++++++++++++-------------
include/linux/filter.h | 6 ++
net/core/filter.c | 4 +
3 files changed, 117 insertions(+), 42 deletions(-)
diff --git a/arch/arm/net/bpf_jit_32.c b/arch/arm/net/bpf_jit_32.c
index 62135849..19d60af 100644
--- a/arch/arm/net/bpf_jit_32.c
+++ b/arch/arm/net/bpf_jit_32.c
@@ -18,6 +18,7 @@
#include <linux/slab.h>
#include <asm/cacheflush.h>
#include <asm/hwcap.h>
+#include <asm/unaligned.h>
#include "bpf_jit_32.h"
@@ -71,7 +72,7 @@ struct jit_ctx {
int bpf_jit_enable __read_mostly;
-static u64 jit_get_skb_b(struct sk_buff *skb, unsigned offset)
+static u64 jit_get_skb_b_pos(struct sk_buff *skb, unsigned offset)
{
u8 ret;
int err;
@@ -81,7 +82,7 @@ static u64 jit_get_skb_b(struct sk_buff *skb, unsigned offset)
return (u64)err << 32 | ret;
}
-static u64 jit_get_skb_h(struct sk_buff *skb, unsigned offset)
+static u64 jit_get_skb_h_pos(struct sk_buff *skb, unsigned offset)
{
u16 ret;
int err;
@@ -91,7 +92,7 @@ static u64 jit_get_skb_h(struct sk_buff *skb, unsigned offset)
return (u64)err << 32 | ntohs(ret);
}
-static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset)
+static u64 jit_get_skb_w_pos(struct sk_buff *skb, unsigned offset)
{
u32 ret;
int err;
@@ -101,6 +102,60 @@ static u64 jit_get_skb_w(struct sk_buff *skb, unsigned offset)
return (u64)err << 32 | ntohl(ret);
}
+static u64 jit_get_skb_b_neg(struct sk_buff *skb, unsigned offset)
+{
+ u8 *ptr;
+
+ ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 1);
+ if (!ptr)
+ return (u64)1 << 32;
+ return *ptr;
+}
+
+static u64 jit_get_skb_h_neg(struct sk_buff *skb, unsigned offset)
+{
+ u16 *ptr;
+
+ ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 2);
+ if (!ptr)
+ return (u64)1 << 32;
+ return get_unaligned_be16(ptr);
+}
+
+static u64 jit_get_skb_w_neg(struct sk_buff *skb, unsigned offset)
+{
+ u32 *ptr;
+
+ ptr = bpf_internal_load_pointer_neg_helper(skb, offset, 4);
+ if (!ptr)
+ return (u64)1 << 32;
+ return get_unaligned_be32(ptr);
+}
+
+static u64 jit_get_skb_b_any(struct sk_buff *skb, unsigned offset)
+{
+ if ((int)offset >= 0)
+ return jit_get_skb_b_pos(skb, offset);
+ else
+ return jit_get_skb_b_neg(skb, offset);
+}
+
+static u64 jit_get_skb_h_any(struct sk_buff *skb, unsigned offset)
+{
+ if ((int)offset >= 0)
+ return jit_get_skb_h_pos(skb, offset);
+ else
+ return jit_get_skb_h_neg(skb, offset);
+}
+
+static u64 jit_get_skb_w_any(struct sk_buff *skb, unsigned offset)
+{
+ if ((int)offset >= 0)
+ return jit_get_skb_w_pos(skb, offset);
+ else
+ return jit_get_skb_w_neg(skb, offset);
+}
+
/*
* Wrapper that handles both OABI and EABI and assures Thumb2 interworking
* (where the assembly routines like __aeabi_uidiv could cause problems).
@@ -458,7 +513,10 @@ static inline void update_on_xread(struct jit_ctx *ctx)
static int build_body(struct jit_ctx *ctx)
{
- void *load_func[] = {jit_get_skb_b, jit_get_skb_h, jit_get_skb_w};
+ void *load_func_any[] = {jit_get_skb_b_any, jit_get_skb_h_any, jit_get_skb_w_any};
+ void *load_func_pos[] = {jit_get_skb_b_pos, jit_get_skb_h_pos, jit_get_skb_w_pos};
+ void *load_func_neg[] = {jit_get_skb_b_neg, jit_get_skb_h_neg, jit_get_skb_w_neg};
+ void **load_func;
const struct sk_filter *prog = ctx->skf;
const struct sock_filter *inst;
unsigned i, load_order, off, condt;
@@ -498,36 +556,38 @@ static int build_body(struct jit_ctx *ctx)
case BPF_S_LD_B_ABS:
load_order = 0;
load:
- /* the interpreter will deal with the negative K */
- if ((int)k < 0)
- return -ENOTSUPP;
emit_mov_i(r_off, k, ctx);
-load_common:
- ctx->seen |= SEEN_DATA | SEEN_CALL;
-
- if (load_order > 0) {
- emit(ARM_SUB_I(r_scratch, r_skb_hl,
- 1 << load_order), ctx);
- emit(ARM_CMP_R(r_scratch, r_off), ctx);
- condt = ARM_COND_HS;
- } else {
- emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
- condt = ARM_COND_HI;
- }
- _emit(condt, ARM_ADD_R(r_scratch, r_off, r_skb_data),
- ctx);
-
- if (load_order == 0)
- _emit(condt, ARM_LDRB_I(r_A, r_scratch, 0),
+ /* deal with negative K */
+ if ((int)k >= 0) {
+ load_func = load_func_pos;
+ if (load_order > 0) {
+ emit(ARM_SUB_I(r_scratch, r_skb_hl,
+ 1 << load_order), ctx);
+ emit(ARM_CMP_R(r_scratch, r_off), ctx);
+ condt = ARM_COND_HS;
+ } else {
+ emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
+ condt = ARM_COND_HI;
+ }
+
+ _emit(condt, ARM_ADD_R(r_scratch, r_off, r_skb_data),
ctx);
- else if (load_order == 1)
- emit_load_be16(condt, r_A, r_scratch, ctx);
- else if (load_order == 2)
- emit_load_be32(condt, r_A, r_scratch, ctx);
- _emit(condt, ARM_B(b_imm(i + 1, ctx)), ctx);
+ if (load_order == 0)
+ _emit(condt, ARM_LDRB_I(r_A, r_scratch, 0),
+ ctx);
+ else if (load_order == 1)
+ emit_load_be16(condt, r_A, r_scratch, ctx);
+ else if (load_order == 2)
+ emit_load_be32(condt, r_A, r_scratch, ctx);
+ _emit(condt, ARM_B(b_imm(i + 1, ctx)), ctx);
+ } else {
+ load_func = load_func_neg;
+ }
+load_common:
+ ctx->seen |= SEEN_DATA | SEEN_CALL;
/* the slowpath */
emit_mov_i(ARM_R3, (u32)load_func[load_order], ctx);
emit(ARM_MOV_R(ARM_R0, r_skb), ctx);
@@ -547,7 +607,9 @@ load_common:
case BPF_S_LD_B_IND:
load_order = 0;
load_ind:
+ load_func = load_func_any;
OP_IMM3(ARM_ADD, r_off, r_X, k, ctx);
+ load_func = load_func_any;
goto load_common;
case BPF_S_LDX_IMM:
ctx->seen |= SEEN_X;
@@ -565,25 +627,28 @@ load_ind:
case BPF_S_LDX_B_MSH:
/* x = ((*(frame + k)) & 0xf) << 2; */
ctx->seen |= SEEN_X | SEEN_DATA | SEEN_CALL;
- /* the interpreter should deal with the negative K */
- if (k < 0)
- return -1;
/* offset in r1: we might have to take the slow path */
emit_mov_i(r_off, k, ctx);
- emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
+ /* deal with negative K */
+ if ((int)k >= 0) {
+ load_func = load_func_pos;
+ emit(ARM_CMP_R(r_skb_hl, r_off), ctx);
- /* load in r0: common with the slowpath */
- _emit(ARM_COND_HI, ARM_LDRB_R(ARM_R0, r_skb_data,
- ARM_R1), ctx);
- /*
- * emit_mov_i() might generate one or two instructions,
- * the same holds for emit_blx_r()
- */
- _emit(ARM_COND_HI, ARM_B(b_imm(i + 1, ctx) - 2), ctx);
+ /* load in r0: common with the slowpath */
+ _emit(ARM_COND_HI, ARM_LDRB_R(ARM_R0, r_skb_data,
+ ARM_R1), ctx);
+ /*
+ * emit_mov_i() might generate one or two instructions,
+ * the same holds for emit_blx_r()
+ */
+ _emit(ARM_COND_HI, ARM_B(b_imm(i + 1, ctx) - 2), ctx);
+ } else {
+ load_func = load_func_neg;
+ }
emit(ARM_MOV_R(ARM_R0, r_skb), ctx);
/* r_off is r1 */
- emit_mov_i(ARM_R3, (u32)jit_get_skb_b, ctx);
+ emit_mov_i(ARM_R3, (u32)load_func[0], ctx);
emit_blx_r(ARM_R3, ctx);
/* check the return value of skb_copy_bits */
emit(ARM_CMP_I(ARM_R1, 0), ctx);
diff --git a/include/linux/filter.h b/include/linux/filter.h
index 8eeb205..78cd56d 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -161,6 +161,12 @@ extern int sk_chk_filter(struct sock_filter *filter, unsigned int flen);
extern void bpf_jit_compile(struct sk_filter *fp);
extern void bpf_jit_free(struct sk_filter *fp);
#define SK_RUN_FILTER(FILTER, SKB) (*FILTER->bpf_func)(SKB, FILTER->insns)
+/*
+ * Only Exported for the bpf jit load helper.
+ * Do not call from anywhere else!
+ */
+extern void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb,
+ int k, unsigned int size);
#else
static inline void bpf_jit_compile(struct sk_filter *fp)
{
diff --git a/net/core/filter.c b/net/core/filter.c
index 6f755cc..9cbaecb 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -42,6 +42,10 @@
/* No hurry in this branch
*
* Exported for the bpf jit load helper.
+ *
+ * CAUTION ! :
+ * If its prototype is ever changed, check arch/{*}/net/{*}.S files,
+ * since it is called from BPF assembly code.
*/
void *bpf_internal_load_pointer_neg_helper(const struct sk_buff *skb, int k, unsigned int size)
{
^ permalink raw reply related
* Re: [PATCH v17 01/15] Add PR_{GET,SET}_NO_NEW_PRIVS to prevent execve from granting privs
From: Markus Gutschke @ 2012-04-06 21:32 UTC (permalink / raw)
To: Andrew Lutomirski
Cc: Andrew Morton, Will Drewry, linux-kernel, linux-security-module,
linux-arch, linux-doc, kernel-hardening, netdev, x86, arnd, davem,
hpa, mingo, oleg, peterz, rdunlap, mcgrathr, tglx, eparis,
serge.hallyn, djm, scarybeasts, indan, pmoore, corbet,
eric.dumazet, coreyb, keescook, jmorris, Andy Lutomirski
In-Reply-To: <CAObL_7E7dHWxKgA7-pSCuUCrxEqY1H55Kxg_sF_OLbdx=pkT4g@mail.gmail.com>
On Fri, Apr 6, 2012 at 14:15, Andrew Lutomirski <luto@mit.edu> wrote:
> Agreed, but I'm talking about something totally different: if I can
> use CLONE_NEWPID, then I can send an unexpected pid with SCM_CREDS.
> The SCM_CREDS receive code should remap pids.
Yes, I know. It's broken. And so is the view of the /proc filesystem
when inside a pid namespace. And things behave funny if you don't set
up a new "init" process inside of the pid namespace. And I am sure, a
few other things are broken that we just haven't run into.
CLONE_NEWPID is tricky. I can understand, if you want to fix it first.
Looking forward to seeing some patches in the future; please cc me, if
you want feedback from an actual user of this code.
The SCM_CREDS issue is the most serious one of the above, but it
doesn't bother me personally, as I would just set up my sandbox policy
to disallow all of SCM_CREDS (*). But that obviously not a good excuse
for leaving a kernel bug around.
Overall, I like both NO_NEW_PRIVS and BPF filters for seccomp though;
they are a great way to reduce the attack surface of the kernel.
Kernel bugs become a lot less of a headache, if I have a way to filter
out the buggy parts of the kernel. It isn't a panacea, but it's a
great new tool to harden applications.
Markus
*) this is currently difficult to filter SCM_CREDS, if we still want
to allow SCM_RIGHTS. See my earlier complaint about sendmsg().
Currently, filtering of sendmsg() probably requires the use of a
helper process.
^ permalink raw reply
* Re: [PATCH v17 00/15] seccomp_filter: BPF-based syscall filtering
From: Andrew Morton @ 2012-04-06 21:28 UTC (permalink / raw)
To: James Morris
Cc: Will Drewry, linux-kernel, linux-security-module, linux-arch,
linux-doc, kernel-hardening, netdev, x86, arnd, davem, hpa, mingo,
oleg, peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn,
djm, scarybeasts, indan, pmoore, corbet, eric.dumazet, markus,
coreyb, keescook, Oleg Nesterov
In-Reply-To: <alpine.LRH.2.02.1203301010310.11201@tundra.namei.org>
On Fri, 30 Mar 2012 10:11:32 +1100 (EST)
James Morris <jmorris@namei.org> wrote:
> On Thu, 29 Mar 2012, Will Drewry wrote:
>
> > Please see prior revisions for a detailed discussion of this patch
> > series.
> >
> > This series is a rebase on to:
> > b5174fa3a7f4f8f150bfa3b917c92608953dfa0f
> > with very minor changes due to rebasing and tweaks noticed by a few
> > initial users. (I will rebase again for v3.4-rc1 when that time comes.)
>
> I've offered to take this via my tree -- if anyone else wants to merge it
> via theirs instead, holler.
OK by me, especially if it means that you'll carefully (re)review it! I
had some minor quibbles with v17.
It would be good if Oleg could take a look at the signal changes, if he
has not yet done so?
^ permalink raw reply
* Re: [PATCH v17 15/15] Documentation: prctl/seccomp_filter
From: Andrew Morton @ 2012-04-06 21:26 UTC (permalink / raw)
To: Will Drewry
Cc: linux-kernel, linux-security-module, linux-arch, linux-doc,
kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
scarybeasts, indan, pmoore, corbet, eric.dumazet, markus, coreyb,
keescook, jmorris
In-Reply-To: <1333051320-30872-16-git-send-email-wad@chromium.org>
On Thu, 29 Mar 2012 15:02:00 -0500
Will Drewry <wad@chromium.org> wrote:
> Documents how system call filtering using Berkeley Packet
> Filter programs works and how it may be used.
> Includes an example for x86 and a semi-generic
> example using a macro-based code generator.
>
>
> ...
>
> +Adding architecture support
> +-----------------------
> +
> +See arch/Kconfig for the authoritative requirements. In general, if an
> +architecture supports both ptrace_event and seccomp, it will be able to
> +support seccomp filter with minor fixup: SIGSYS support and seccomp return
> +value checking. Then it must just add CONFIG_HAVE_ARCH_SECCOMP_FILTER
> +to its arch-specific Kconfig.
> diff --git a/samples/Makefile b/samples/Makefile
> index 2f75851..5ef08bb 100644
> --- a/samples/Makefile
> +++ b/samples/Makefile
Oh good, I was going to ask about that.
Can we get this code into tools/testing/selftests? That way people
will run it more often and it's more likely to be maintained as the
code evolves.
^ permalink raw reply
* Re: [PATCH v17 13/15] ptrace,seccomp: Add PTRACE_SECCOMP support
From: Andrew Morton @ 2012-04-06 21:24 UTC (permalink / raw)
To: Will Drewry
Cc: linux-kernel, linux-security-module, linux-arch, linux-doc,
kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
scarybeasts, indan, pmoore, corbet, eric.dumazet, markus, coreyb,
keescook, jmorris
In-Reply-To: <1333051320-30872-14-git-send-email-wad@chromium.org>
On Thu, 29 Mar 2012 15:01:58 -0500
Will Drewry <wad@chromium.org> wrote:
> This change adds support for a new ptrace option, PTRACE_O_TRACESECCOMP,
> and a new return value for seccomp BPF programs, SECCOMP_RET_TRACE.
>
> When a tracer specifies the PTRACE_O_TRACESECCOMP ptrace option, the
> tracer will be notified, via PTRACE_EVENT_SECCOMP, for any syscall that
> results in a BPF program returning SECCOMP_RET_TRACE. The 16-bit
> SECCOMP_RET_DATA mask of the BPF program return value will be passed as
> the ptrace_message and may be retrieved using PTRACE_GETEVENTMSG.
>
> If the subordinate process is not using seccomp filter, then no
> system call notifications will occur even if the option is specified.
>
> If there is no tracer with PTRACE_O_TRACESECCOMP when SECCOMP_RET_TRACE
> is returned, the system call will not be executed and an -ENOSYS errno
> will be returned to userspace.
>
> This change adds a dependency on the system call slow path. Any future
> efforts to use the system call fast path for seccomp filter will need to
> address this restriction.
>
>
> ...
>
> @@ -410,6 +411,15 @@ int __secure_computing_int(int this_syscall)
> /* Let the filter pass back 16 bits of data. */
> seccomp_send_sigsys(this_syscall, data);
> goto skip;
> + case SECCOMP_RET_TRACE:
> + /* Skip these calls if there is no tracer. */
> + if (!ptrace_event_enabled(current, PTRACE_EVENT_SECCOMP))
> + goto skip;
> + /* Allow the BPF to provide the event message */
> + ptrace_event(PTRACE_EVENT_SECCOMP, data);
> + if (fatal_signal_pending(current))
> + break;
I don't have all the patches applied here so the context is missing.
Perhaps tht would help me understand what this fatal_signal_pending()
test is doing here. But an explanatory comment wouldn't hurt.
What *is* it here for, anyway?
> + return 0;
> case SECCOMP_RET_ALLOW:
> return 0;
> case SECCOMP_RET_KILL:
^ permalink raw reply
* Re: Expose ltr/obff interface by sysfs
From: Don Dutile @ 2012-04-06 21:20 UTC (permalink / raw)
To: Jesse Barnes
Cc: Hao, Xudong, linux-pci@vger.kernel.org, netdev@vger.kernel.org,
e1000-devel@lists.sourceforge.net
In-Reply-To: <20120406082649.68fe5842@jbarnes-desktop>
On 04/06/2012 11:26 AM, Jesse Barnes wrote:
> On Fri, 6 Apr 2012 02:43:59 +0000
> "Hao, Xudong"<xudong.hao@intel.com> wrote:
>
>> Hi,
>>
>> I'm working on virtualization Xen/KVM. I saw there are ltr/obff enabling/disabling function in pci.c, but no called till now. I want to know if anybody(driver developer) are working for using it? Can driver change the LTR latency value dynamically?
>
> I believe the value is writable, but I'd expect some devices to
> misbehave if the value were programmed too high. Performance would
> also suffer if the value were set too high, at least for IOPS sensitive
> devices.
>
>> /*
>> LTR(Latency tolerance reporting) allows devices to send messages to the root complex indicating their latency tolerance for snooped& unsnooped memory transactions.
>> OBFF (optimized buffer flush/fill), where supported, can help improve energy efficiency by giving devices information about when interrupts and other activity will have a reduced power impact.
>> */
>>
>> One way to control ltr/obff is used by driver, however, I'm considering that in virtualization, how guest OS driver control them. I have an idea that expose an inode interface by sysfs, like "reset" inode implemented in pci-sysfs.c, so that system user/administrator can enable/disable ltr/obff or set latency value on userspace, but not limited on driver. Comments?
>
> Given how device specific these extensions are, I'd expect you'd need
> to know about each specific device anyway, which is why I think the
> control belongs in the driver. I don't see why you'd need to
> enable/disable/change these functions when assigning a device from one
> guest to another...
>
and looking at this code last week, I noticed the ltr, obff & ido
functions should be doing pcie_cap_has_*() checks before
accessing registers that may not exist, as is done in
pci_[save,restore]_pcie_state() ...
in drivers/pci/pci.c:
#define pcie_cap_has_devctl2(type, flags) \
((flags & PCI_EXP_FLAGS_VERS) > 1)
#define pcie_cap_has_lnkctl2(type, flags) \
((flags & PCI_EXP_FLAGS_VERS) > 1)
#define pcie_cap_has_sltctl2(type, flags) \
((flags & PCI_EXP_FLAGS_VERS) > 1)
^ permalink raw reply
* Re: [PATCH v17 10/15] seccomp: add SECCOMP_RET_ERRNO
From: Andrew Morton @ 2012-04-06 21:19 UTC (permalink / raw)
To: Will Drewry
Cc: linux-kernel, linux-security-module, linux-arch, linux-doc,
kernel-hardening, netdev, x86, arnd, davem, hpa, mingo, oleg,
peterz, rdunlap, mcgrathr, tglx, luto, eparis, serge.hallyn, djm,
scarybeasts, indan, pmoore, corbet, eric.dumazet, markus, coreyb,
keescook, jmorris
In-Reply-To: <1333051320-30872-11-git-send-email-wad@chromium.org>
On Thu, 29 Mar 2012 15:01:55 -0500
Will Drewry <wad@chromium.org> wrote:
> This change adds the SECCOMP_RET_ERRNO as a valid return value from a
> seccomp filter. Additionally, it makes the first use of the lower
> 16-bits for storing a filter-supplied errno. 16-bits is more than
> enough for the errno-base.h calls.
>
> Returning errors instead of immediately terminating processes that
> violate seccomp policy allow for broader use of this functionality
> for kernel attack surface reduction. For example, a linux container
> could maintain a whitelist of pre-existing system calls but drop
> all new ones with errnos. This would keep a logically static attack
> surface while providing errnos that may allow for graceful failure
> without the downside of do_exit() on a bad call.
>
>
> ...
>
> @@ -64,11 +65,17 @@ struct seccomp {
> struct seccomp_filter *filter;
> };
>
> -extern void __secure_computing(int);
> -static inline void secure_computing(int this_syscall)
> +/*
> + * Direct callers to __secure_computing should be updated as
> + * CONFIG_HAVE_ARCH_SECCOMP_FILTER propagates.
Are there any such callers? There's one I see in arm, but it's called
from assembly code.
> + */
> +extern void __secure_computing(int) __deprecated;
> +extern int __secure_computing_int(int);
> +static inline int secure_computing(int this_syscall)
> {
> if (unlikely(test_thread_flag(TIF_SECCOMP)))
> - __secure_computing(this_syscall);
> + return __secure_computing_int(this_syscall);
> + return 0;
> }
>
> ...
>
> void __secure_computing(int this_syscall)
> {
> + /* Filter calls should never use this function. */
> + BUG_ON(current->seccomp.mode == SECCOMP_MODE_FILTER);
> + __secure_computing_int(this_syscall);
> +}
> +
> +int __secure_computing_int(int this_syscall)
What the heck does "_int" mean here? I read it as "integer" but
perhaps it's shorthand for "internal". Give us a better name, please.
Or a code comment.
> +{
> int mode = current->seccomp.mode;
> int exit_sig = 0;
> int *syscall;
> + u32 ret = SECCOMP_RET_KILL;
> + int data;
>
> switch (mode) {
> case SECCOMP_MODE_STRICT:
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox