* Re: use-after-free in usbnet
From: Ming Lei @ 2012-04-22 12:05 UTC (permalink / raw)
To: Huajun Li
Cc: Oliver Neukum, Alan Stern, Dave Jones, netdev, linux-usb,
Fedora Kernel Team
In-Reply-To: <CA+v9cxaqL1V3eyv=dhKbqbnXk6wm1qm93DNFbrbwbd1b1X+3rw@mail.gmail.com>
On Sun, Apr 22, 2012 at 10:19 AM, Huajun Li <huajun.li.lee@gmail.com> wrote:
> On Sat, Apr 21, 2012 at 3:45 PM, Ming Lei <tom.leiming@gmail.com> wrote:
>> Hi Huajun,
>>
>> On Sat, Apr 21, 2012 at 3:06 PM, Ming Lei <tom.leiming@gmail.com> wrote:
>>>> Just skip trying this per your following email's comments.
>>>
>>> I will prepare a new patch later, if you'd like to try it.
>>
>> The below patch reverts the below commits:
>>
>> 0956a8c20b23d429e79ff86d4325583fc06f9eb4
>> (usbnet: increase URB reference count before usb_unlink_urb)
>>
>> 4231d47e6fe69f061f96c98c30eaf9fb4c14b96d
>> (net/usbnet: avoid recursive locking in usbnet_stop())
>>
>> and keep holding tx/rx queue lock during unlinking, but avoid
>> to acquire the same queue lock inside complete handler triggered by
>> usb_unlink_urb.
>>
>> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
>> index db99536..effb34e 100644
>> --- a/drivers/net/usb/usbnet.c
>> +++ b/drivers/net/usb/usbnet.c
>> @@ -291,9 +291,11 @@ static void defer_bh(struct usbnet *dev, struct
>> sk_buff *skb, struct sk_buff_hea
>> {
>> unsigned long flags;
>>
>> - spin_lock_irqsave(&list->lock, flags);
>> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
>> + spin_lock_irqsave(&list->lock, flags);
>> __skb_unlink(skb, list);
>> - spin_unlock(&list->lock);
>> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
>> + spin_unlock(&list->lock);
>> spin_lock(&dev->done.lock);
>> __skb_queue_tail(&dev->done, skb);
>> if (dev->done.qlen == 1)
>
>
> Then 'flags' may not be initialized, and this will cause problem while
> calling spin_unlock_irqrestore(&dev->done.lock, flags), right?
The flag is a percpu variable, so it can't change during the
above code piece.
>
>
>> @@ -345,7 +347,8 @@ static int rx_submit (struct usbnet *dev, struct
>> urb *urb, gfp_t flags)
>> usb_fill_bulk_urb (urb, dev->udev, dev->in,
>> skb->data, size, rx_complete, skb);
>>
>> - spin_lock_irqsave (&dev->rxq.lock, lockflags);
>> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
>> + spin_lock_irqsave (&dev->rxq.lock, lockflags);
>>
>> if (netif_running (dev->net) &&
>> netif_device_present (dev->net) &&
>> @@ -377,7 +380,8 @@ static int rx_submit (struct usbnet *dev, struct
>> urb *urb, gfp_t flags)
>> netif_dbg(dev, ifdown, dev->net, "rx: stopped\n");
>> retval = -ENOLINK;
>> }
>> - spin_unlock_irqrestore (&dev->rxq.lock, lockflags);
>> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
>> + spin_unlock_irqrestore (&dev->rxq.lock, lockflags);
>> if (retval) {
>> dev_kfree_skb_any (skb);
>> usb_free_urb (urb);
>> @@ -582,6 +586,7 @@ static int unlink_urbs (struct usbnet *dev, struct
>> sk_buff_head *q)
>> int count = 0;
>>
>> spin_lock_irqsave (&q->lock, flags);
>> + set_cpu_bit(URB_UNLINKING, dev->cpuflags);
>> skb_queue_walk_safe(q, skb, skbnext) {
>> struct skb_data *entry;
>> struct urb *urb;
>> @@ -590,15 +595,6 @@ static int unlink_urbs (struct usbnet *dev,
>> struct sk_buff_head *q)
>> entry = (struct skb_data *) skb->cb;
>> urb = entry->urb;
>>
>> - /*
>> - * Get reference count of the URB to avoid it to be
>> - * freed during usb_unlink_urb, which may trigger
>> - * use-after-free problem inside usb_unlink_urb since
>> - * usb_unlink_urb is always racing with .complete
>> - * handler(include defer_bh).
>> - */
>> - usb_get_urb(urb);
>> - spin_unlock_irqrestore(&q->lock, flags);
>> // during some PM-driven resume scenarios,
>> // these (async) unlinks complete immediately
>> retval = usb_unlink_urb (urb);
>> @@ -606,9 +602,8 @@ static int unlink_urbs (struct usbnet *dev, struct
>> sk_buff_head *q)
>> netdev_dbg(dev->net, "unlink urb err, %d\n", retval);
>> else
>> count++;
>> - usb_put_urb(urb);
>> - spin_lock_irqsave(&q->lock, flags);
>> }
>> + clear_cpu_bit(URB_UNLINKING, dev->cpuflags);
>> spin_unlock_irqrestore (&q->lock, flags);
>> return count;
>> }
>> @@ -1283,6 +1278,7 @@ void usbnet_disconnect (struct usb_interface *intf)
>> usb_kill_urb(dev->interrupt);
>> usb_free_urb(dev->interrupt);
>>
>> + free_percpu(dev->cpuflags);
>> free_netdev(net);
>> usb_put_dev (xdev);
>> }
>> @@ -1353,6 +1349,13 @@ usbnet_probe (struct usb_interface *udev, const
>> struct usb_device_id *prod)
>> SET_NETDEV_DEV(net, &udev->dev);
>>
>> dev = netdev_priv(net);
>> +
>> + dev->cpuflags = alloc_percpu(unsigned long);
>> + if (!dev->cpuflags) {
>> + status = -ENOMEM;
>> + goto out1;
>> + }
>> +
>> dev->udev = xdev;
>> dev->intf = udev;
>> dev->driver_info = info;
>> @@ -1396,7 +1399,7 @@ usbnet_probe (struct usb_interface *udev, const
>> struct usb_device_id *prod)
>> if (info->bind) {
>> status = info->bind (dev, udev);
>> if (status < 0)
>> - goto out1;
>> + goto out2;
>>
>> // heuristic: "usb%d" for links we know are two-host,
>> // else "eth%d" when there's reasonable doubt. userspace
>> @@ -1465,6 +1468,8 @@ usbnet_probe (struct usb_interface *udev, const
>> struct usb_device_id *prod)
>> out3:
>> if (info->unbind)
>> info->unbind (dev, udev);
>> +out2:
>> + free_percpu(dev->cpuflags);
>> out1:
>> free_netdev(net);
>> out:
>> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
>> index 605b0aa..2dc46f5 100644
>> --- a/include/linux/usb/usbnet.h
>> +++ b/include/linux/usb/usbnet.h
>> @@ -69,8 +69,28 @@ struct usbnet {
>> # define EVENT_DEV_WAKING 6
>> # define EVENT_DEV_ASLEEP 7
>> # define EVENT_DEV_OPEN 8
>> + unsigned long __percpu *cpuflags;
>> +# define URB_UNLINKING 0
>
> Is it possible using a simple bool variable to track whether q->lock
> is hold by unlink_urb() ? If yes, it can avoid introducing following
> new codes into current code stack.
It should be defined as percpu variable. The URB complete handler
may be triggered inside unlink path, or it can be triggered in hardirq path
from other CPUs at the same time.
>
>> };
>>
>> +static inline void set_cpu_bit(int nr, unsigned long __percpu *addr)
>> +{
>> + unsigned long *fl = __this_cpu_ptr(addr);
>> + set_bit(nr, fl);
>> +}
>> +
>> +static inline void clear_cpu_bit(int nr, unsigned long __percpu *addr)
>> +{
>> + unsigned long *fl = __this_cpu_ptr(addr);
>> + clear_bit(nr, fl);
>> +}
>> +
>> +static inline int test_cpu_bit(int nr, unsigned long __percpu *addr)
>> +{
>> + unsigned long *fl = __this_cpu_ptr(addr);
>> + return test_bit(nr, fl);
>> +}
>> +
>> static inline struct usb_driver *driver_of(struct usb_interface *intf)
>> {
>> return to_usb_driver(intf->dev.driver);
>>
>>
>> Thanks,
>> --
>> Ming Lei
--
Ming Lei
^ permalink raw reply
* Re: [PATCH] NET: bcm63xx_enet: move phy_(dis)connect into probe/remove
From: Jonas Gorski @ 2012-04-22 11:31 UTC (permalink / raw)
To: mbizon, Andy Fleming
Cc: netdev, Florian Fainelli, Eric Dumazet, David S. Miller
In-Reply-To: <1334852238.5185.40.camel@sakura.staff.proxad.net>
On 19 April 2012 18:17, Maxime Bizon <mbizon@freebox.fr> wrote:
>
> On Thu, 2012-04-19 at 16:52 +0200, Jonas Gorski wrote:
>
>> Yes, but none of the ethtool functions cause register writes in the
>> priv->has_phy = true case when in PHY_READY or PHY_HALTED state. All
>> they do is modify the phy_device's settings.
>
> unless I'm mistaken:
>
> phy_ethtool_sset() => phy_start_aneg()
>
> will kick the state machine even when state is PHY_READY
Hmm. I see what you mean. I wonder if it is intended that you can do
that without having phy_start() called first.
@Andy, can you perhaps shed some light on this? How are ethernet
drivers supposed to behave/when should they call
phy_connect()/phy_start()? Currently most drivers call phy_connect()
in their _probe(), and phy_start() in _open(), so many seem to have
the issue that the phy state machine is in PHY_READY after _probe(),
and can be kicked into running through ethtool even if the interface
is down.
This problem goes away after the first ifup/ifdown cycle, since the
phy state machine is then in PHY_HALTED, which gets properly caught in
phy_start_aneg().
To me it looks like phy_start_aneg() should check for some more
states, as it currently would also overwrite a PHY_STARTING or
PHY_PENDING state, which looks definitely wrong to me.
Jonas
^ permalink raw reply
* ..A.U Alluvial gold dust for sale..
From: Nathan Bako @ 2012-04-21 16:24 UTC (permalink / raw)
To: netdev
Dear sir / madam,
I am in possession of 500 Kg alluvial gold dust of 22,80 carats which I want to sale on the international market being a little small scale miner in the field, I seek for a buyer or person likely to receive the product and to find a purchaser.
Initially, I intend to offer a quantity for a first transaction then the remaining will follow gradually. The selling price is very interesting, contact me if you are interested.
Thanks,
Mr. Nathan Bako.
^ permalink raw reply
* Web teknisk support team
From: Webmaster Upgradeing Avdeling @ 2012-04-22 9:31 UTC (permalink / raw)
Kjære Webmail medlem ®.
Denne meldingen er fra vårt sikre webmaster.
Kjære medlem, er Denne e å varsle deg om at de er
tiden pågår vedlikehold for webmaster oppgradering til en
Internett-tjeneste sikrere og raskere. vi å måtte oppdage at de fleste
ikke har vært aktiv for litt tid på grunn av en eller annen grunn, blir vi
eliminere inntekt Vår viktigste server for å redusere byrden, slik det har
vært ubrukelig.Vi foreslår at du hjelper oss med å beskytte din konto.
Hvis du mottar denne meldingen betyr at din e-post har blitt kø for off:
dette var et resultat av en kontinuerlig skriptfeil (kode: 505) har
mottatt e-post adresse dette problemet og vi er i ferd med å slette
kontoen. For å kontrollere at posten er aktiv, må du svare på denne e-post
med følgende
informasjon om bekreftelse.
Epost Gjeldende Brukernavn: {}
Epost Gjeldende passord: {}
Reconfirm Passord: {}
Merk: Å gi uriktige opplysninger eller ignorerer dette
Meldingen vil bli løst i deaktivering av din e-post
post fra vårt kontrollpanel.
Takk © Web-mail. Alle rettigheter reservert.
Web teknisk support team
e-postvarsling
^ permalink raw reply
* Re: rx_dropped packets stop with tcpdump running
From: Bill Fink @ 2012-04-22 5:13 UTC (permalink / raw)
To: Marco Berizzi; +Cc: brian.haley, tushar.n.dave, netdev, eric.dumazet
In-Reply-To: <DUB108-W57C2F8FCF145C20E3B8BF1B2220@phx.gbl>
On Fri, 20 Apr 2012 17:25:28 +0200, Marco Berizzi wrote:
> brian.haley@hp.com wrote:
> > 0x886d or 0x86DD? That second one is IPv6, you might just need to load the module.
>
> here is the tcpdump:
>
> 17:23:25.561398 00:19:99:90:ba:72 > ff:ff:ff:ff:ff:ff, ethertype Unknown (0x886d), length 64:
> 0x0000: 0001 0001 000c 24e2 0001 0019 9990 ba72 ......$........r
> 0x0010: 0000 0500 2304 6364 726f 447b d700 80f8 ....#.cdroD{....
> 0x0020: ffff 20d1 b631 80fa ffff e02b fb30 80fa .....1.....+.0..
> 0x0030: ffff ..
> 17:23:25.561402 00:19:99:90:ba:73 > ff:ff:ff:ff:ff:ff, ethertype Unknown (0x886d), length 64:
> 0x0000: 0001 0001 000c 24e2 0002 0019 9990 ba72 ......$........r
> 0x0010: 6700 5400 6800 7200 6500 7300 6800 6f00 g.T.h.r.e.s.h.o.
> 0x0020: 6c00 6400 0000 3700 3200 0000 0000 0000 l.d...7.2.......
> 0x0030: 0000 ..
FYI, a Google search for "ethertype 886d" revealed that this is
supposedly an Intel ANS probe. See:
http://fixunix.com/networking/337443-packets-broadcasting-every-second.html
-Bill
^ permalink raw reply
* Re: Question with secure_ipv4_port_ephemeral() implementation
From: Tetsuo Handa @ 2012-04-22 5:20 UTC (permalink / raw)
To: bhutchings; +Cc: netdev
In-Reply-To: <1335065393.3209.361.camel@deadeye>
Ben Hutchings wrote:
> As I understand it, that 8-bit counter was used for all connections, so
> in order to spoof the source of a TCP connection it was only necessary
> to guess 24 bits of the ISN. On a sufficiently fast network, it would
> now be feasible to carry out a brute force attack that ACKs all possible
> ISNs before the handshake times-out. That's not yet feasible if the
> attacker has to guess all 32 bits of the ISN.
So, the purpose was to make the initial sequence number more random. OK.
> The original reason for periodically regenerating the secret was that
> the hash function was quite weak and the secret could presumably be
> found in a reasonably short time. So, without regeneration, the hash
> also has to be stronger.
My concern is the purpose of making the automatic local port number selection
algorithm less random. That commit removed uptime factor from factors that
determine starting point of available local port scanning (due to removal of
periodic get_random_bytes() calls).
368 static inline u32 inet_sk_port_offset(const struct sock *sk)
369 {
370 const struct inet_sock *inet = inet_sk(sk);
371 return secure_ipv4_port_ephemeral(inet->inet_rcv_saddr,
372 inet->inet_daddr,
373 inet->inet_dport);
374 }
secure_ipv4_port_ephemeral() no longer depends on uptime.
565 int inet_hash_connect(struct inet_timewait_death_row *death_row,
566 struct sock *sk)
567 {
568 return __inet_hash_connect(death_row, sk, inet_sk_port_offset(sk),
569 __inet_check_established, __inet_hash_nolisten);
570 }
inet_sk_port_offset() no longer depends on uptime.
It returns same port offset for same addresses.
454 int __inet_hash_connect(struct inet_timewait_death_row *death_row,
455 struct sock *sk, u32 port_offset,
456 int (*check_established)(struct inet_timewait_death_row *,
457 struct sock *, __u16, struct inet_timewait_sock **),
458 int (*hash)(struct sock *sk, struct inet_timewait_sock *twp))
459 {
460 struct inet_hashinfo *hinfo = death_row->hashinfo;
461 const unsigned short snum = inet_sk(sk)->inet_num;
462 struct inet_bind_hashbucket *head;
463 struct inet_bind_bucket *tb;
464 int ret;
465 struct net *net = sock_net(sk);
466 int twrefcnt = 1;
467
468 if (!snum) {
469 int i, remaining, low, high, port;
470 static u32 hint;
471 u32 offset = hint + port_offset;
port_offset no longer depends on uptime.
472 struct hlist_node *node;
473 struct inet_timewait_sock *tw = NULL;
474
475 inet_get_local_port_range(&low, &high);
476 remaining = (high - low) + 1;
477
478 local_bh_disable();
479 for (i = 1; i <= remaining; i++) {
480 port = low + (i + offset) % remaining;
That commit changed to scan available local port independent with uptime.
481 if (inet_is_reserved_local_port(port))
482 continue;
I worried we unexpectedly made the automatic local port number selection
algorithm less random. If we expectedly made this algorithm less random,
I wanted to know whether there was a reason we should not depend on
uptime factor.
^ permalink raw reply
* [PATCH net-next] be2net: fix ethtool get settings
From: Ajit Khaparde @ 2012-04-22 4:53 UTC (permalink / raw)
To: davem; +Cc: netdev
ethtool get settings was not displaying all the settings correctly.
use the get_phy_info to get more information about the PHY to fix this.
Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com>
---
drivers/net/ethernet/emulex/benet/be.h | 23 ++-
drivers/net/ethernet/emulex/benet/be_cmds.c | 17 +-
drivers/net/ethernet/emulex/benet/be_cmds.h | 36 ++++-
drivers/net/ethernet/emulex/benet/be_ethtool.c | 245 ++++++++++++++++--------
drivers/net/ethernet/emulex/benet/be_main.c | 20 +--
5 files changed, 239 insertions(+), 102 deletions(-)
diff --git a/drivers/net/ethernet/emulex/benet/be.h b/drivers/net/ethernet/emulex/benet/be.h
index 9576ac0..ad69cf8 100644
--- a/drivers/net/ethernet/emulex/benet/be.h
+++ b/drivers/net/ethernet/emulex/benet/be.h
@@ -313,6 +313,23 @@ struct be_vf_cfg {
#define BE_UC_PMAC_COUNT 30
#define BE_VF_UC_PMAC_COUNT 2
+struct phy_info {
+ u8 transceiver;
+ u8 autoneg;
+ u8 fc_autoneg;
+ u8 port_type;
+ u16 phy_type;
+ u16 interface_type;
+ u32 misc_params;
+ u16 auto_speeds_supported;
+ u16 fixed_speeds_supported;
+ int link_speed;
+ int forced_port_speed;
+ u32 dac_cable_len;
+ u32 advertising;
+ u32 supported;
+};
+
struct be_adapter {
struct pci_dev *pdev;
struct net_device *netdev;
@@ -377,10 +394,6 @@ struct be_adapter {
u32 rx_fc; /* Rx flow control */
u32 tx_fc; /* Tx flow control */
bool stats_cmd_sent;
- int link_speed;
- u8 port_type;
- u8 transceiver;
- u8 autoneg;
u8 generation; /* BladeEngine ASIC generation */
u32 flash_status;
struct completion flash_compl;
@@ -392,6 +405,7 @@ struct be_adapter {
u32 sli_family;
u8 hba_port_num;
u16 pvid;
+ struct phy_info phy;
u8 wol_cap;
bool wol;
u32 max_pmac_cnt; /* Max secondary UC MACs programmable */
@@ -583,4 +597,5 @@ extern void be_link_status_update(struct be_adapter *adapter, u8 link_status);
extern void be_parse_stats(struct be_adapter *adapter);
extern int be_load_fw(struct be_adapter *adapter, u8 *func);
extern bool be_is_wol_supported(struct be_adapter *adapter);
+extern bool be_pause_supported(struct be_adapter *adapter);
#endif /* BE_H */
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.c b/drivers/net/ethernet/emulex/benet/be_cmds.c
index 67b030d..22be08c 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.c
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.c
@@ -126,7 +126,7 @@ static void be_async_link_state_process(struct be_adapter *adapter,
struct be_async_event_link_state *evt)
{
/* When link status changes, link speed must be re-queried from FW */
- adapter->link_speed = -1;
+ adapter->phy.link_speed = -1;
/* For the initial link status do not rely on the ASYNC event as
* it may not be received in some cases.
@@ -153,7 +153,7 @@ static void be_async_grp5_qos_speed_process(struct be_adapter *adapter,
{
if (evt->physical_port == adapter->port_num) {
/* qos_link_speed is in units of 10 Mbps */
- adapter->link_speed = evt->qos_link_speed * 10;
+ adapter->phy.link_speed = evt->qos_link_speed * 10;
}
}
@@ -2136,8 +2136,7 @@ err:
return status;
}
-int be_cmd_get_phy_info(struct be_adapter *adapter,
- struct be_phy_info *phy_info)
+int be_cmd_get_phy_info(struct be_adapter *adapter)
{
struct be_mcc_wrb *wrb;
struct be_cmd_req_get_phy_info *req;
@@ -2170,9 +2169,15 @@ int be_cmd_get_phy_info(struct be_adapter *adapter,
if (!status) {
struct be_phy_info *resp_phy_info =
cmd.va + sizeof(struct be_cmd_req_hdr);
- phy_info->phy_type = le16_to_cpu(resp_phy_info->phy_type);
- phy_info->interface_type =
+ adapter->phy.phy_type = le16_to_cpu(resp_phy_info->phy_type);
+ adapter->phy.interface_type =
le16_to_cpu(resp_phy_info->interface_type);
+ adapter->phy.auto_speeds_supported =
+ le16_to_cpu(resp_phy_info->auto_speeds_supported);
+ adapter->phy.fixed_speeds_supported =
+ le16_to_cpu(resp_phy_info->fixed_speeds_supported);
+ adapter->phy.misc_params =
+ le32_to_cpu(resp_phy_info->misc_params);
}
pci_free_consistent(adapter->pdev, cmd.size,
cmd.va, cmd.dma);
diff --git a/drivers/net/ethernet/emulex/benet/be_cmds.h b/drivers/net/ethernet/emulex/benet/be_cmds.h
index d5b680c..3c54361 100644
--- a/drivers/net/ethernet/emulex/benet/be_cmds.h
+++ b/drivers/net/ethernet/emulex/benet/be_cmds.h
@@ -1309,9 +1309,36 @@ enum {
PHY_TYPE_KX4_10GB,
PHY_TYPE_BASET_10GB,
PHY_TYPE_BASET_1GB,
+ PHY_TYPE_BASEX_1GB,
+ PHY_TYPE_SGMII,
PHY_TYPE_DISABLED = 255
};
+#define BE_SUPPORTED_SPEED_NONE 0
+#define BE_SUPPORTED_SPEED_10MBPS 1
+#define BE_SUPPORTED_SPEED_100MBPS 2
+#define BE_SUPPORTED_SPEED_1GBPS 4
+#define BE_SUPPORTED_SPEED_10GBPS 8
+
+#define BE_AN_EN 0x2
+#define BE_PAUSE_SYM_EN 0x80
+
+/* MAC speed valid values */
+#define SPEED_DEFAULT 0x0
+#define SPEED_FORCED_10GB 0x1
+#define SPEED_FORCED_1GB 0x2
+#define SPEED_AUTONEG_10GB 0x3
+#define SPEED_AUTONEG_1GB 0x4
+#define SPEED_AUTONEG_100MB 0x5
+#define SPEED_AUTONEG_10GB_1GB 0x6
+#define SPEED_AUTONEG_10GB_1GB_100MB 0x7
+#define SPEED_AUTONEG_1GB_100MB 0x8
+#define SPEED_AUTONEG_10MB 0x9
+#define SPEED_AUTONEG_1GB_100MB_10MB 0xa
+#define SPEED_AUTONEG_100MB_10MB 0xb
+#define SPEED_FORCED_100MB 0xc
+#define SPEED_FORCED_10MB 0xd
+
struct be_cmd_req_get_phy_info {
struct be_cmd_req_hdr hdr;
u8 rsvd0[24];
@@ -1321,7 +1348,11 @@ struct be_phy_info {
u16 phy_type;
u16 interface_type;
u32 misc_params;
- u32 future_use[4];
+ u16 ext_phy_details;
+ u16 rsvd;
+ u16 auto_speeds_supported;
+ u16 fixed_speeds_supported;
+ u32 future_use[2];
};
struct be_cmd_resp_get_phy_info {
@@ -1655,8 +1686,7 @@ extern int be_cmd_get_seeprom_data(struct be_adapter *adapter,
struct be_dma_mem *nonemb_cmd);
extern int be_cmd_set_loopback(struct be_adapter *adapter, u8 port_num,
u8 loopback_type, u8 enable);
-extern int be_cmd_get_phy_info(struct be_adapter *adapter,
- struct be_phy_info *phy_info);
+extern int be_cmd_get_phy_info(struct be_adapter *adapter);
extern int be_cmd_set_qos(struct be_adapter *adapter, u32 bps, u32 domain);
extern void be_detect_dump_ue(struct be_adapter *adapter);
extern int be_cmd_get_die_temperature(struct be_adapter *adapter);
diff --git a/drivers/net/ethernet/emulex/benet/be_ethtool.c b/drivers/net/ethernet/emulex/benet/be_ethtool.c
index c1ff73c..dc9f74c 100644
--- a/drivers/net/ethernet/emulex/benet/be_ethtool.c
+++ b/drivers/net/ethernet/emulex/benet/be_ethtool.c
@@ -433,102 +433,193 @@ static int be_get_sset_count(struct net_device *netdev, int stringset)
}
}
+static u32 be_get_port_type(u32 phy_type, u32 dac_cable_len)
+{
+ u32 port;
+
+ switch (phy_type) {
+ case PHY_TYPE_BASET_1GB:
+ case PHY_TYPE_BASEX_1GB:
+ case PHY_TYPE_SGMII:
+ port = PORT_TP;
+ break;
+ case PHY_TYPE_SFP_PLUS_10GB:
+ port = dac_cable_len ? PORT_DA : PORT_FIBRE;
+ break;
+ case PHY_TYPE_XFP_10GB:
+ case PHY_TYPE_SFP_1GB:
+ port = PORT_FIBRE;
+ break;
+ case PHY_TYPE_BASET_10GB:
+ port = PORT_TP;
+ break;
+ default:
+ port = PORT_OTHER;
+ }
+
+ return port;
+}
+
+static u32 convert_to_et_setting(u32 if_type, u32 if_speeds)
+{
+ u32 val = 0;
+
+ switch (if_type) {
+ case PHY_TYPE_BASET_1GB:
+ case PHY_TYPE_BASEX_1GB:
+ case PHY_TYPE_SGMII:
+ val |= SUPPORTED_TP;
+ if (if_speeds & BE_SUPPORTED_SPEED_1GBPS)
+ val |= SUPPORTED_1000baseT_Full;
+ if (if_speeds & BE_SUPPORTED_SPEED_100MBPS)
+ val |= SUPPORTED_100baseT_Full;
+ if (if_speeds & BE_SUPPORTED_SPEED_10MBPS)
+ val |= SUPPORTED_10baseT_Full;
+ break;
+ case PHY_TYPE_KX4_10GB:
+ val |= SUPPORTED_Backplane;
+ if (if_speeds & BE_SUPPORTED_SPEED_1GBPS)
+ val |= SUPPORTED_1000baseKX_Full;
+ if (if_speeds & BE_SUPPORTED_SPEED_10GBPS)
+ val |= SUPPORTED_10000baseKX4_Full;
+ break;
+ case PHY_TYPE_KR_10GB:
+ val |= SUPPORTED_Backplane |
+ SUPPORTED_10000baseKR_Full;
+ break;
+ case PHY_TYPE_SFP_PLUS_10GB:
+ case PHY_TYPE_XFP_10GB:
+ case PHY_TYPE_SFP_1GB:
+ val |= SUPPORTED_FIBRE;
+ if (if_speeds & BE_SUPPORTED_SPEED_10GBPS)
+ val |= SUPPORTED_10000baseT_Full;
+ if (if_speeds & BE_SUPPORTED_SPEED_1GBPS)
+ val |= SUPPORTED_1000baseT_Full;
+ break;
+ case PHY_TYPE_BASET_10GB:
+ val |= SUPPORTED_TP;
+ if (if_speeds & BE_SUPPORTED_SPEED_10GBPS)
+ val |= SUPPORTED_10000baseT_Full;
+ if (if_speeds & BE_SUPPORTED_SPEED_1GBPS)
+ val |= SUPPORTED_1000baseT_Full;
+ if (if_speeds & BE_SUPPORTED_SPEED_100MBPS)
+ val |= SUPPORTED_100baseT_Full;
+ break;
+ default:
+ val |= SUPPORTED_TP;
+ }
+
+ return val;
+}
+
+static int convert_to_et_speed(u32 be_speed)
+{
+ int et_speed = SPEED_10000;
+
+ switch (be_speed) {
+ case PHY_LINK_SPEED_10MBPS:
+ et_speed = SPEED_10;
+ break;
+ case PHY_LINK_SPEED_100MBPS:
+ et_speed = SPEED_100;
+ break;
+ case PHY_LINK_SPEED_1GBPS:
+ et_speed = SPEED_1000;
+ break;
+ case PHY_LINK_SPEED_10GBPS:
+ et_speed = SPEED_10000;
+ break;
+ }
+
+ return et_speed;
+}
+
+bool be_pause_supported(struct be_adapter *adapter)
+{
+ return (adapter->phy.interface_type == PHY_TYPE_SFP_PLUS_10GB ||
+ adapter->phy.interface_type == PHY_TYPE_XFP_10GB) ?
+ false : true;
+}
+
static int be_get_settings(struct net_device *netdev, struct ethtool_cmd *ecmd)
{
struct be_adapter *adapter = netdev_priv(netdev);
- struct be_phy_info phy_info;
- u8 mac_speed = 0;
+ u8 port_speed = 0;
u16 link_speed = 0;
u8 link_status;
+ u32 et_speed = 0;
int status;
- if ((adapter->link_speed < 0) || (!(netdev->flags & IFF_UP))) {
- status = be_cmd_link_status_query(adapter, &mac_speed,
- &link_speed, &link_status, 0);
- if (!status)
- be_link_status_update(adapter, link_status);
-
- /* link_speed is in units of 10 Mbps */
- if (link_speed) {
- ethtool_cmd_speed_set(ecmd, link_speed*10);
+ if (adapter->phy.link_speed < 0 || !(netdev->flags & IFF_UP)) {
+ if (adapter->phy.forced_port_speed < 0) {
+ status = be_cmd_link_status_query(adapter, &port_speed,
+ &link_speed, &link_status, 0);
+ if (!status)
+ be_link_status_update(adapter, link_status);
+ if (link_speed)
+ et_speed = link_speed;
+ else
+ et_speed = convert_to_et_speed(port_speed);
} else {
- switch (mac_speed) {
- case PHY_LINK_SPEED_10MBPS:
- ethtool_cmd_speed_set(ecmd, SPEED_10);
- break;
- case PHY_LINK_SPEED_100MBPS:
- ethtool_cmd_speed_set(ecmd, SPEED_100);
- break;
- case PHY_LINK_SPEED_1GBPS:
- ethtool_cmd_speed_set(ecmd, SPEED_1000);
- break;
- case PHY_LINK_SPEED_10GBPS:
- ethtool_cmd_speed_set(ecmd, SPEED_10000);
- break;
- case PHY_LINK_SPEED_ZERO:
- ethtool_cmd_speed_set(ecmd, 0);
- break;
- }
+ et_speed = adapter->phy.forced_port_speed;
}
- status = be_cmd_get_phy_info(adapter, &phy_info);
- if (!status) {
- switch (phy_info.interface_type) {
- case PHY_TYPE_XFP_10GB:
- case PHY_TYPE_SFP_1GB:
- case PHY_TYPE_SFP_PLUS_10GB:
- ecmd->port = PORT_FIBRE;
- break;
- default:
- ecmd->port = PORT_TP;
- break;
- }
+ ethtool_cmd_speed_set(ecmd, et_speed);
+
+ status = be_cmd_get_phy_info(adapter);
+ if (status)
+ return status;
+
+ ecmd->supported =
+ convert_to_et_setting(adapter->phy.interface_type,
+ adapter->phy.auto_speeds_supported |
+ adapter->phy.fixed_speeds_supported);
+ ecmd->advertising =
+ convert_to_et_setting(adapter->phy.interface_type,
+ adapter->phy.auto_speeds_supported);
- switch (phy_info.interface_type) {
- case PHY_TYPE_KR_10GB:
- case PHY_TYPE_KX4_10GB:
- ecmd->autoneg = AUTONEG_ENABLE;
+ ecmd->port = be_get_port_type(adapter->phy.interface_type,
+ adapter->phy.dac_cable_len);
+
+ if (adapter->phy.auto_speeds_supported) {
+ ecmd->supported |= SUPPORTED_Autoneg;
+ ecmd->autoneg = AUTONEG_ENABLE;
+ ecmd->advertising |= ADVERTISED_Autoneg;
+ }
+
+ if (be_pause_supported(adapter)) {
+ ecmd->supported |= SUPPORTED_Pause;
+ ecmd->advertising |= ADVERTISED_Pause;
+ }
+
+ switch (adapter->phy.interface_type) {
+ case PHY_TYPE_KR_10GB:
+ case PHY_TYPE_KX4_10GB:
ecmd->transceiver = XCVR_INTERNAL;
- break;
- default:
- ecmd->autoneg = AUTONEG_DISABLE;
- ecmd->transceiver = XCVR_EXTERNAL;
- break;
- }
+ break;
+ default:
+ ecmd->transceiver = XCVR_EXTERNAL;
+ break;
}
/* Save for future use */
- adapter->link_speed = ethtool_cmd_speed(ecmd);
- adapter->port_type = ecmd->port;
- adapter->transceiver = ecmd->transceiver;
- adapter->autoneg = ecmd->autoneg;
+ adapter->phy.link_speed = ethtool_cmd_speed(ecmd);
+ adapter->phy.port_type = ecmd->port;
+ adapter->phy.transceiver = ecmd->transceiver;
+ adapter->phy.autoneg = ecmd->autoneg;
+ adapter->phy.advertising = ecmd->advertising;
+ adapter->phy.supported = ecmd->supported;
} else {
- ethtool_cmd_speed_set(ecmd, adapter->link_speed);
- ecmd->port = adapter->port_type;
- ecmd->transceiver = adapter->transceiver;
- ecmd->autoneg = adapter->autoneg;
+ ethtool_cmd_speed_set(ecmd, adapter->phy.link_speed);
+ ecmd->port = adapter->phy.port_type;
+ ecmd->transceiver = adapter->phy.transceiver;
+ ecmd->autoneg = adapter->phy.autoneg;
+ ecmd->advertising = adapter->phy.advertising;
+ ecmd->supported = adapter->phy.supported;
}
ecmd->duplex = DUPLEX_FULL;
ecmd->phy_address = adapter->port_num;
- switch (ecmd->port) {
- case PORT_FIBRE:
- ecmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_FIBRE);
- break;
- case PORT_TP:
- ecmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_TP);
- break;
- case PORT_AUI:
- ecmd->supported = (SUPPORTED_10000baseT_Full | SUPPORTED_AUI);
- break;
- }
-
- if (ecmd->autoneg) {
- ecmd->supported |= SUPPORTED_1000baseT_Full;
- ecmd->supported |= SUPPORTED_Autoneg;
- ecmd->advertising |= (ADVERTISED_10000baseT_Full |
- ADVERTISED_1000baseT_Full);
- }
return 0;
}
@@ -548,7 +639,7 @@ be_get_pauseparam(struct net_device *netdev, struct ethtool_pauseparam *ecmd)
struct be_adapter *adapter = netdev_priv(netdev);
be_cmd_get_flow_control(adapter, &ecmd->tx_pause, &ecmd->rx_pause);
- ecmd->autoneg = 0;
+ ecmd->autoneg = adapter->phy.fc_autoneg;
}
static int
diff --git a/drivers/net/ethernet/emulex/benet/be_main.c b/drivers/net/ethernet/emulex/benet/be_main.c
index 528a886..a5bc608 100644
--- a/drivers/net/ethernet/emulex/benet/be_main.c
+++ b/drivers/net/ethernet/emulex/benet/be_main.c
@@ -2571,11 +2571,12 @@ err:
static void be_setup_init(struct be_adapter *adapter)
{
adapter->vlan_prio_bmap = 0xff;
- adapter->link_speed = -1;
+ adapter->phy.link_speed = -1;
adapter->if_handle = -1;
adapter->be3_native = false;
adapter->promiscuous = false;
adapter->eq_next_idx = 0;
+ adapter->phy.forced_port_speed = -1;
}
static int be_add_mac_from_list(struct be_adapter *adapter, u8 *mac)
@@ -2707,6 +2708,10 @@ static int be_setup(struct be_adapter *adapter)
goto err;
}
+ be_cmd_get_phy_info(adapter);
+ if (be_pause_supported(adapter))
+ adapter->phy.fc_autoneg = 1;
+
schedule_delayed_work(&adapter->work, msecs_to_jiffies(1000));
adapter->flags |= BE_FLAGS_WORKER_SCHEDULED;
@@ -2760,17 +2765,8 @@ static bool be_flash_redboot(struct be_adapter *adapter,
static bool phy_flashing_required(struct be_adapter *adapter)
{
- int status = 0;
- struct be_phy_info phy_info;
-
- status = be_cmd_get_phy_info(adapter, &phy_info);
- if (status)
- return false;
- if ((phy_info.phy_type == TN_8022) &&
- (phy_info.interface_type == PHY_TYPE_BASET_10GB)) {
- return true;
- }
- return false;
+ return (adapter->phy.phy_type == TN_8022 &&
+ adapter->phy.interface_type == PHY_TYPE_BASET_10GB);
}
static int be_flash_data(struct be_adapter *adapter,
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 00/10] l2tp: misc fixes and add L2TPv3 IP encap over IPv6
From: David Miller @ 2012-04-22 4:29 UTC (permalink / raw)
To: jchapman; +Cc: netdev, bcrl
In-Reply-To: <1334912573-28804-1-git-send-email-jchapman@katalix.com>
From: James Chapman <jchapman@katalix.com>
Date: Fri, 20 Apr 2012 10:02:43 +0100
> This patch series includes several L2TP fixes / cleanups and adds
> L2TPv3 IP encapsulation support for IPv6.
>
> Patches 4-10 depend on Benjamin LaHaise's L2TP UDP IPv6 patches which
> have already been submitted for review. Patches 1-3 are not IPv6
> related so can be reviewed / applied now.
They aren't in review state, they're in "changed requested" state
because they are obviously broken and need changes since you hit
all of those checksum problems on loopback when you tested it.
Never submit patch sets against another patch series that is going
to have to go through changes before it goes into the tree.
I'm dropping this entire set, resubmit after Ben fixes the checksum
problems and does a new submission.
^ permalink raw reply
* Re: Question with secure_ipv4_port_ephemeral() implementation
From: Ben Hutchings @ 2012-04-22 3:29 UTC (permalink / raw)
To: Tetsuo Handa; +Cc: netdev
In-Reply-To: <201204202130.IJF95312.tQJFLMFOFSVHOO@I-love.SAKURA.ne.jp>
On Fri, 2012-04-20 at 21:30 +0900, Tetsuo Handa wrote:
> Commit 6e5714ea "net: Compute protocol sequence numbers and fragment IDs using
> MD5." removed periodic get_random_bytes() calls. After that commit,
>
> static u32 net_secret[MD5_MESSAGE_BYTES / 4] ____cacheline_aligned;
>
> is filled with random bytes for only once upon boot and is never updated again.
> Then, shouldn't net_secret be marked as __read_mostly?
>
> Just from curiosity... what was the reason for changing
> secure_ipv4_port_ephemeral() generate same return value for same arguments?
> Was periodically changing return value for same arguments unfriendly with NAT
> or something?
The commit message says:
> Furthermore, only having 24-bits of the sequence number be truly
> unpredictable is a very serious limitation. So the periodic
> regeneration and 8-bit counter have been removed. We compute and
> use a full 32-bit sequence number.
As I understand it, that 8-bit counter was used for all connections, so
in order to spoof the source of a TCP connection it was only necessary
to guess 24 bits of the ISN. On a sufficiently fast network, it would
now be feasible to carry out a brute force attack that ACKs all possible
ISNs before the handshake times-out. That's not yet feasible if the
attacker has to guess all 32 bits of the ISN.
The original reason for periodically regenerating the secret was that
the hash function was quite weak and the secret could presumably be
found in a reasonably short time. So, without regeneration, the hash
also has to be stronger.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next 12/19] net neighbour: Convert to use register_net_sysctl
From: Ben Hutchings @ 2012-04-22 2:36 UTC (permalink / raw)
To: Eric W. Biederman
Cc: Pavel Emelyanov, David Miller, netdev@vger.kernel.org,
Serge E. Hallyn, Gao feng, pablo@netfilter.org, Stephen Hemminger
In-Reply-To: <m1ty0ec1bg.fsf@fess.ebiederm.org>
On Fri, 2012-04-20 at 00:25 -0700, Eric W. Biederman wrote:
> Pavel Emelyanov <xemul@parallels.com> writes:
>
> >> @@ -2925,19 +2924,7 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p,
> >> {
> >> struct neigh_sysctl_table *t;
> >> const char *dev_name_source = NULL;
> >> -
> >> -#define NEIGH_CTL_PATH_ROOT 0
> >> -#define NEIGH_CTL_PATH_PROTO 1
> >> -#define NEIGH_CTL_PATH_NEIGH 2
> >> -#define NEIGH_CTL_PATH_DEV 3
> >> -
> >> - struct ctl_path neigh_path[] = {
> >> - { .procname = "net", },
> >> - { .procname = "proto", },
> >> - { .procname = "neigh", },
> >> - { .procname = "default", },
> >> - { },
> >> - };
> >> + char neigh_path[ sizeof("net//neigh/") + IFNAMSIZ + IFNAMSIZ ];
> >
> > Why two IFNAMSIZ-es? One is for the dev->name, but the other one is not.
> > Is it just for not having any other better constant at hands?
>
> Yep. We don't seem to have any proto name size constants, and all
> of decnet ipv4 and ipv6 are all shorter than the 16 bytes of IFNAMSIZ.
I don't think it makes any sense to put in IFNAMSIZ as a size for a
string that isn't a device name.
> Even if I am wrong the snprintf below truncates it's output to the
> buffer size and null terminates it so in the worst case we won't cause
> a buffer overflow, we will just get a truncated path name to pass
> to sysctl.
>
> Shrug I stopped at good enough but I am happy for a better number.
Truncation by snprintf() is definitely better than overflow, but we
should also check and WARN so that if someone breaks this it's hard to
miss.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: use-after-free in usbnet
From: Huajun Li @ 2012-04-22 2:19 UTC (permalink / raw)
To: Ming Lei
Cc: Oliver Neukum, Alan Stern, Dave Jones,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Fedora Kernel Team
In-Reply-To: <CACVXFVNc_S8pTaBqMzQZx6Dt-tSP_9iXepxJzv=iR9BFu=Tj8g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Apr 21, 2012 at 3:45 PM, Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Huajun,
>
> On Sat, Apr 21, 2012 at 3:06 PM, Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Just skip trying this per your following email's comments.
>>
>> I will prepare a new patch later, if you'd like to try it.
>
> The below patch reverts the below commits:
>
> 0956a8c20b23d429e79ff86d4325583fc06f9eb4
> (usbnet: increase URB reference count before usb_unlink_urb)
>
> 4231d47e6fe69f061f96c98c30eaf9fb4c14b96d
> (net/usbnet: avoid recursive locking in usbnet_stop())
>
> and keep holding tx/rx queue lock during unlinking, but avoid
> to acquire the same queue lock inside complete handler triggered by
> usb_unlink_urb.
>
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index db99536..effb34e 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -291,9 +291,11 @@ static void defer_bh(struct usbnet *dev, struct
> sk_buff *skb, struct sk_buff_hea
> {
> unsigned long flags;
>
> - spin_lock_irqsave(&list->lock, flags);
> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
> + spin_lock_irqsave(&list->lock, flags);
> __skb_unlink(skb, list);
> - spin_unlock(&list->lock);
> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
> + spin_unlock(&list->lock);
> spin_lock(&dev->done.lock);
> __skb_queue_tail(&dev->done, skb);
> if (dev->done.qlen == 1)
Then 'flags' may not be initialized, and this will cause problem while
calling spin_unlock_irqrestore(&dev->done.lock, flags), right?
> @@ -345,7 +347,8 @@ static int rx_submit (struct usbnet *dev, struct
> urb *urb, gfp_t flags)
> usb_fill_bulk_urb (urb, dev->udev, dev->in,
> skb->data, size, rx_complete, skb);
>
> - spin_lock_irqsave (&dev->rxq.lock, lockflags);
> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
> + spin_lock_irqsave (&dev->rxq.lock, lockflags);
>
> if (netif_running (dev->net) &&
> netif_device_present (dev->net) &&
> @@ -377,7 +380,8 @@ static int rx_submit (struct usbnet *dev, struct
> urb *urb, gfp_t flags)
> netif_dbg(dev, ifdown, dev->net, "rx: stopped\n");
> retval = -ENOLINK;
> }
> - spin_unlock_irqrestore (&dev->rxq.lock, lockflags);
> + if (!test_cpu_bit(URB_UNLINKING, dev->cpuflags))
> + spin_unlock_irqrestore (&dev->rxq.lock, lockflags);
> if (retval) {
> dev_kfree_skb_any (skb);
> usb_free_urb (urb);
> @@ -582,6 +586,7 @@ static int unlink_urbs (struct usbnet *dev, struct
> sk_buff_head *q)
> int count = 0;
>
> spin_lock_irqsave (&q->lock, flags);
> + set_cpu_bit(URB_UNLINKING, dev->cpuflags);
> skb_queue_walk_safe(q, skb, skbnext) {
> struct skb_data *entry;
> struct urb *urb;
> @@ -590,15 +595,6 @@ static int unlink_urbs (struct usbnet *dev,
> struct sk_buff_head *q)
> entry = (struct skb_data *) skb->cb;
> urb = entry->urb;
>
> - /*
> - * Get reference count of the URB to avoid it to be
> - * freed during usb_unlink_urb, which may trigger
> - * use-after-free problem inside usb_unlink_urb since
> - * usb_unlink_urb is always racing with .complete
> - * handler(include defer_bh).
> - */
> - usb_get_urb(urb);
> - spin_unlock_irqrestore(&q->lock, flags);
> // during some PM-driven resume scenarios,
> // these (async) unlinks complete immediately
> retval = usb_unlink_urb (urb);
> @@ -606,9 +602,8 @@ static int unlink_urbs (struct usbnet *dev, struct
> sk_buff_head *q)
> netdev_dbg(dev->net, "unlink urb err, %d\n", retval);
> else
> count++;
> - usb_put_urb(urb);
> - spin_lock_irqsave(&q->lock, flags);
> }
> + clear_cpu_bit(URB_UNLINKING, dev->cpuflags);
> spin_unlock_irqrestore (&q->lock, flags);
> return count;
> }
> @@ -1283,6 +1278,7 @@ void usbnet_disconnect (struct usb_interface *intf)
> usb_kill_urb(dev->interrupt);
> usb_free_urb(dev->interrupt);
>
> + free_percpu(dev->cpuflags);
> free_netdev(net);
> usb_put_dev (xdev);
> }
> @@ -1353,6 +1349,13 @@ usbnet_probe (struct usb_interface *udev, const
> struct usb_device_id *prod)
> SET_NETDEV_DEV(net, &udev->dev);
>
> dev = netdev_priv(net);
> +
> + dev->cpuflags = alloc_percpu(unsigned long);
> + if (!dev->cpuflags) {
> + status = -ENOMEM;
> + goto out1;
> + }
> +
> dev->udev = xdev;
> dev->intf = udev;
> dev->driver_info = info;
> @@ -1396,7 +1399,7 @@ usbnet_probe (struct usb_interface *udev, const
> struct usb_device_id *prod)
> if (info->bind) {
> status = info->bind (dev, udev);
> if (status < 0)
> - goto out1;
> + goto out2;
>
> // heuristic: "usb%d" for links we know are two-host,
> // else "eth%d" when there's reasonable doubt. userspace
> @@ -1465,6 +1468,8 @@ usbnet_probe (struct usb_interface *udev, const
> struct usb_device_id *prod)
> out3:
> if (info->unbind)
> info->unbind (dev, udev);
> +out2:
> + free_percpu(dev->cpuflags);
> out1:
> free_netdev(net);
> out:
> diff --git a/include/linux/usb/usbnet.h b/include/linux/usb/usbnet.h
> index 605b0aa..2dc46f5 100644
> --- a/include/linux/usb/usbnet.h
> +++ b/include/linux/usb/usbnet.h
> @@ -69,8 +69,28 @@ struct usbnet {
> # define EVENT_DEV_WAKING 6
> # define EVENT_DEV_ASLEEP 7
> # define EVENT_DEV_OPEN 8
> + unsigned long __percpu *cpuflags;
> +# define URB_UNLINKING 0
Is it possible using a simple bool variable to track whether q->lock
is hold by unlink_urb() ? If yes, it can avoid introducing following
new codes into current code stack.
> };
>
> +static inline void set_cpu_bit(int nr, unsigned long __percpu *addr)
> +{
> + unsigned long *fl = __this_cpu_ptr(addr);
> + set_bit(nr, fl);
> +}
> +
> +static inline void clear_cpu_bit(int nr, unsigned long __percpu *addr)
> +{
> + unsigned long *fl = __this_cpu_ptr(addr);
> + clear_bit(nr, fl);
> +}
> +
> +static inline int test_cpu_bit(int nr, unsigned long __percpu *addr)
> +{
> + unsigned long *fl = __this_cpu_ptr(addr);
> + return test_bit(nr, fl);
> +}
> +
> static inline struct usb_driver *driver_of(struct usb_interface *intf)
> {
> return to_usb_driver(intf->dev.driver);
>
>
> Thanks,
> --
> Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: use-after-free in usbnet
From: David Miller @ 2012-04-22 2:05 UTC (permalink / raw)
To: huajun.li.lee
Cc: tom.leiming, oneukum, stern, davej, netdev, linux-usb,
kernel-team
In-Reply-To: <CA+v9cxbdQW1idnxes95vHMS+j2XPFw4fDfaDLbJa7enMnyq6LQ@mail.gmail.com>
From: Huajun Li <huajun.li.lee@gmail.com>
Date: Sun, 22 Apr 2012 09:45:55 +0800
> On Sun, Apr 22, 2012 at 3:23 AM, David Miller <davem@davemloft.net> wrote:
>> From: Ming Lei <tom.leiming@gmail.com>
>> Date: Sat, 21 Apr 2012 09:49:51 +0800
>>
>>> I see the problem, so looks skb_queue_walk_safe is not safe.
>>> I don' know why the 2nd ' tmp = skb->next' in skb_queue_walk_safe
>>> is needed and it may become unsafe if skb is freed during current loop.
>>
>> I can't see what the problem is, skb_queue_walk_safe() is perfect
>> and does exactly what it advertises to do.
>>
>> If 'skb' is unlinked inside of an skb_queue_walk_safe() loop, that's
>> fine, because we won't touch 'skb' in the loop iteration tail code.
>>
>> Instead, before the loop contents, we pre-fetch skb->next into 'tmp'
>> and then at the end we move 'skb' forward by simply assigning 'tmp'.
>
> In this case, the problem is, 'tmp = skb->next' can be moved out of
> rxq/txq, and even be freed. Then in next loop cycle, 'skb = tmp' will
> refer to a freed skb. You know, in current code stack, unlink_urbs()
> releases q->lock in each loop, this gives chance to urb complete
> handler to call defer_bh() and cause the problem.
Right, just like interfaces such as list_for_each_entry_safe(), this
macro isn't designed to handle cases where you unlink more than one
entry in the list. Specifically, it's designed only to handle the
case when you unlink the entry being processed in the current loop
iteration.
^ permalink raw reply
* Re: use-after-free in usbnet
From: Huajun Li @ 2012-04-22 2:02 UTC (permalink / raw)
To: Ming Lei
Cc: Oliver Neukum, Alan Stern, Dave Jones,
netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA,
Fedora Kernel Team
In-Reply-To: <CACVXFVOemJqfT9OPRer3qzbVEsGyUOupoOUNCBzC4deNRsksQw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Sat, Apr 21, 2012 at 5:40 PM, Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> Hi Huajun,
>
> On Sat, Apr 21, 2012 at 4:23 PM, Huajun Li <huajun.li.lee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> On Sat, Apr 21, 2012 at 3:56 PM, Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>> Hi Huajun,
>>>
>>> On Sat, Apr 21, 2012 at 3:50 PM, Huajun Li <huajun.li.lee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>>>
>>>> Did we on the same page, could you please review my patch again?
>>>>
>>>> My draft patch was based on current mainline( 3.4.0-rc3) which had
>>>> already integrated your previous patch. And in my patch, it replaced
>>>> skb_queue_walk_safe() with skb_queue_walk(), so you will not see 'tmp
>>>> = skb->next' any more.
>>>
>>> Replace skb_queue_walk_safe with skb_queue_walk doesn't improve
>>> the problem, since 'skb = skb->next' in skb_queue_walk still may trigger
>>> the oops, does it?
>>>
>>
>> No.
>> In each loop, my patch traverse the queue from its head, and it always
>> holds q->lock when it need refer "skb->next", this can make sure the
>> right skb is not moved out of rxq/txq.
>
> OK, your patch can avoid the oops, sorry for miss the point.
>
>>
>> Can this fix what you concern? If so, IMO, there is no need to revert
>> your previous patch.
>
> But your patch may introduce another problem, in fact, what your patch does
> is basically same with the below change[1]:
>
> So we can find easily that one same URB may be unlinked more than one
> time with your patch because usb_unlink_urb is asynchronous even though
> it behaves synchronously sometimes.
>
> I remembered that is not allowed, at least usb_unlink_urb's comment says so:
>
> URBs complete only once per submission, and may be canceled only
> once per submission.
>
Yes, this is a problem should be avoided.
> [1], against 3.4.0-rc3
> diff --git a/drivers/net/usb/usbnet.c b/drivers/net/usb/usbnet.c
> index db99536..aadf009 100644
> --- a/drivers/net/usb/usbnet.c
> +++ b/drivers/net/usb/usbnet.c
> @@ -578,15 +578,19 @@ EXPORT_SYMBOL_GPL(usbnet_purge_paused_rxq);
> static int unlink_urbs (struct usbnet *dev, struct sk_buff_head *q)
> {
> unsigned long flags;
> - struct sk_buff *skb, *skbnext;
> + struct sk_buff *skb;
> int count = 0;
>
> spin_lock_irqsave (&q->lock, flags);
> - skb_queue_walk_safe(q, skb, skbnext) {
> + while (1) {
> struct skb_data *entry;
> struct urb *urb;
> int retval;
>
> + skb = q->next;
> + if (skb == (struct sk_buff *)q)
> + break;
> +
> entry = (struct skb_data *) skb->cb;
> urb = entry->urb;
>
>
> Thanks,
> --
> Ming Lei
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: use-after-free in usbnet
From: Huajun Li @ 2012-04-22 1:45 UTC (permalink / raw)
To: David Miller
Cc: tom.leiming-Re5JQEeQqe8AvxtiuMwx3w, oneukum-l3A5Bk7waGM,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
davej-H+wXaHxf7aLQT0dZR+AlfA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
kernel-team-rxtnV0ftBwyoClj4AeEUq9i2O/JbrIOy
In-Reply-To: <20120421.152345.290988116097275353.davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org>
On Sun, Apr 22, 2012 at 3:23 AM, David Miller <davem-fT/PcQaiUtIeIZ0/mPfg9Q@public.gmane.org> wrote:
> From: Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> Date: Sat, 21 Apr 2012 09:49:51 +0800
>
>> I see the problem, so looks skb_queue_walk_safe is not safe.
>> I don' know why the 2nd ' tmp = skb->next' in skb_queue_walk_safe
>> is needed and it may become unsafe if skb is freed during current loop.
>
> I can't see what the problem is, skb_queue_walk_safe() is perfect
> and does exactly what it advertises to do.
>
> If 'skb' is unlinked inside of an skb_queue_walk_safe() loop, that's
> fine, because we won't touch 'skb' in the loop iteration tail code.
>
> Instead, before the loop contents, we pre-fetch skb->next into 'tmp'
> and then at the end we move 'skb' forward by simply assigning 'tmp'.
In this case, the problem is, 'tmp = skb->next' can be moved out of
rxq/txq, and even be freed. Then in next loop cycle, 'skb = tmp' will
refer to a freed skb. You know, in current code stack, unlink_urbs()
releases q->lock in each loop, this gives chance to urb complete
handler to call defer_bh() and cause the problem.
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Server Rental Service in HK
From: boris @ 2012-04-22 0:42 UTC (permalink / raw)
Dear All,
We have our own datacenter in Hong Kong & provide email/application/web rental service to clients.We are APNIC member & provide clean IP to clients.
Dell? PowerEdge? EnterpriseRack Mount Server
-Intel(R) Xeon(R) E3-1240 Processor (3.3GHz, 8M Cache, Turbo, 4C/8T, 80W)
-8GB RAM, 2x4GB, 1333MHz, DDR-3, Dual Ranked UDIMMs
-500GB, 3.5", 6Gbps SAS x 2
-Raid 1 Mirroring Protection
-Remote KVM (iDRAC6 Enterprise)
Dell(TM) PowerEdge(TM) R410 Rack Mount Server
-Intel(R) Quad Core E5606 Xeon(R) CPU, 2.13GHz, 4M Cache, 4.86 GT/s QPI
-4GB Memory (2x2GB), 1333MHz Dual Ranked RDIMMs Fully-Buffered
-500GB 7.2K RPM SATAII 3.5" Hard Drive x 2
-iDRAC6 Enterprise or Express (Remote KVM Management)
Every Dedicated Server Hosting Solution Also Includes:
Software Specification
- CentOS / Fedora / Debian / FreeBSD / Ubuntu / Redhat Linux
- Full root-level access
- Data Center Facilities
- Shared Local & International Bandwidth
- 2 IP Addresses Allocation
- Un-interruptible Power Supply (UPS) backed up by private diesel generator
- FM200¡§based fire suppression system
- 24x7 CRAC Air Conditioning and Humidity Control
- 24x7 Security Control
- 24x7 Remote Hand Service
Pls send us email for further information.Thanks,
Boris
boris@dedicatedserver.com.hk
If you do not wish to further receive this event message, email "borislamsv2@gmail.com" to unsubscribe this message or remove your email from the list.
^ permalink raw reply
* Re: [PATCH v5 2/3] netdev/of/phy: Add MDIO bus multiplexer support.
From: David Miller @ 2012-04-21 23:03 UTC (permalink / raw)
To: david.s.daney
Cc: ddaney.cavm, grant.likely, rob.herring, devicetree-discuss,
netdev, linux-kernel, linux-mips, afleming, galak, david.daney
In-Reply-To: <4F9325DF.7020003@gmail.com>
From: David Daney <david.s.daney@gmail.com>
Date: Sat, 21 Apr 2012 14:25:51 -0700
> If we were to specify the dependencies in both places, we gain nothing
> other than duplication of information.
Each Kconfig option enabling a module has to have appropriate
dependencies.
^ permalink raw reply
* [PATCH] [brcmsmac] "INTERMEDIATE but not AMPDU" only when tracing
From: Eldad Zack @ 2012-04-21 22:48 UTC (permalink / raw)
To: Brett Rudley, Roland Vossen, Arend van Spriel, Kan Yan,
John W. Linville
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Eldad Zack
I keep getting the following messages on the log buffer:
[ 2167.097507] ieee80211 phy0: brcms_c_dotxstatus: INTERMEDIATE but not AMPDU
[ 2281.331305] ieee80211 phy0: brcms_c_dotxstatus: INTERMEDIATE but not AMPDU
[ 2281.332539] ieee80211 phy0: brcms_c_dotxstatus: INTERMEDIATE but not AMPDU
[ 2329.876605] ieee80211 phy0: brcms_c_dotxstatus: INTERMEDIATE but not AMPDU
[ 2329.877354] ieee80211 phy0: brcms_c_dotxstatus: INTERMEDIATE but not AMPDU
[ 2462.280756] ieee80211 phy0: brcms_c_dotxstatus: INTERMEDIATE but not AMPDU
[ 2615.651689] ieee80211 phy0: brcms_c_dotxstatus: INTERMEDIATE but not AMPDU
>From the code comment I understand that this something that can - and does, quite frequently - happen.
This patch demotes the message to the trace level.
Signed-off-by: Eldad Zack <eldad-v6AqZgAe4C4dvzyIwGCCOg@public.gmane.org>
---
drivers/net/wireless/brcm80211/brcmsmac/main.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/net/wireless/brcm80211/brcmsmac/main.c b/drivers/net/wireless/brcm80211/brcmsmac/main.c
index 231ddf4..2fc5956 100644
--- a/drivers/net/wireless/brcm80211/brcmsmac/main.c
+++ b/drivers/net/wireless/brcm80211/brcmsmac/main.c
@@ -847,8 +847,7 @@ brcms_c_dotxstatus(struct brcms_c_info *wlc, struct tx_status *txs)
*/
if (!(txs->status & TX_STATUS_AMPDU)
&& (txs->status & TX_STATUS_INTERMEDIATE)) {
- wiphy_err(wlc->wiphy, "%s: INTERMEDIATE but not AMPDU\n",
- __func__);
+ BCMMSG(wlc->wiphy, "INTERMEDIATE but not AMPDU\n");
return false;
}
--
1.7.9.5
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH v5 2/3] netdev/of/phy: Add MDIO bus multiplexer support.
From: David Daney @ 2012-04-21 21:25 UTC (permalink / raw)
To: David Miller
Cc: ddaney.cavm, grant.likely, rob.herring, devicetree-discuss,
netdev, linux-kernel, linux-mips, afleming, galak, david.daney
In-Reply-To: <20120421.153201.2103447307695063734.davem@davemloft.net>
On 04/21/2012 12:32 PM, David Miller wrote:
> From: David Daney<ddaney.cavm@gmail.com>
> Date: Wed, 18 Apr 2012 16:20:53 -0700
>
>> +config MDIO_BUS_MUX
>> + tristate
>> + help
>> + This module provides a driver framework for MDIO bus
>> + multiplexers which connect one of several child MDIO busses
>> + to a parent bus. Switching between child busses is done by
>> + device specific drivers.
>> +
> This driver uses OF and OF_MDIO, and therefore need dependencies upon
> them. Otherwise it can be enabled in configurations which will result
> in build failures.
Note that this symbol cannot be selected by the user, only indirectly
via the MDIO_BUS_MUX_GPIO symbol (in patch 3/3) which has the proper
dependencies.
If we were to specify the dependencies in both places, we gain nothing other than duplication of information.
However, if you insist, I will of course add it here too.
David Daney
^ permalink raw reply
* Re: [PATCH] gianfar: add GRO support
From: David Miller @ 2012-04-21 20:44 UTC (permalink / raw)
To: b06378; +Cc: netdev, linuxppc-dev
In-Reply-To: <1334912075-27073-1-git-send-email-b06378@freescale.com>
From: Jiajun Wu <b06378@freescale.com>
Date: Fri, 20 Apr 2012 16:54:35 +0800
> Replace netif_receive_skb with napi_gro_receive.
>
> Signed-off-by: Jiajun Wu <b06378@freescale.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] be2net: fix ethtool get settings
From: David Miller @ 2012-04-21 20:38 UTC (permalink / raw)
To: ajit.khaparde; +Cc: netdev
In-Reply-To: <20120419212124.GA31818@akhaparde-VBox>
From: Ajit Khaparde <ajit.khaparde@emulex.com>
Date: Thu, 19 Apr 2012 16:21:24 -0500
> Please apply to net-next.
This doesn't belong in the body of the commit message.
> Signed-off-by: Ajit Khaparde <ajit.khaparde@emulex.com>
Please resubmit this, but with a commit message which describes
what exactly was wrong with ethtool get setting in this driver,
and how you fixed it.
^ permalink raw reply
* Re: [PATCH net-next] tcp: move duplicate code from tcp_v4_init_sock()/tcp_v6_init_sock()
From: David Miller @ 2012-04-21 20:31 UTC (permalink / raw)
To: ncardwell
Cc: netdev, edumazet, nanditad, ycheng, ilpo.jarvinen, maze, therbert
In-Reply-To: <1334865321-20288-1-git-send-email-ncardwell@google.com>
From: Neal Cardwell <ncardwell@google.com>
Date: Thu, 19 Apr 2012 15:55:21 -0400
> This commit moves the (substantial) common code shared between
> tcp_v4_init_sock() and tcp_v6_init_sock() to a new address-family
> independent function, tcp_init_sock().
>
> Centralizing this functionality should help avoid drift issues,
> e.g. where the IPv4 side is updated without a corresponding update to
> IPv6. There was already some drift: IPv4 initialized snd_cwnd to
> TCP_INIT_CWND, while the IPv6 side was still initializing snd_cwnd to
> 2 (in this case it should not matter, since snd_cwnd is also
> initialized in tcp_init_metrics(), but the general risks and
> maintenance overhead remain).
>
> When diffing the old and new code, note that new tcp_init_sock()
> function uses the order of steps from the tcp_v4_init_sock()
> implementation (the order is slightly different in
> tcp_v6_init_sock()).
>
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] af_packet: packet_getsockopt() cleanup
From: David Miller @ 2012-04-21 20:31 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1334908571.2395.228.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Fri, 20 Apr 2012 09:56:11 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> Factorize code, since most fetched values are int type.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] net: allow better page reuse in splice(sock -> pipe)
From: David Miller @ 2012-04-21 20:31 UTC (permalink / raw)
To: eric.dumazet; +Cc: netdev
In-Reply-To: <1334864297.2395.220.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 19 Apr 2012 21:38:17 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> splice() from socket to pipe needs linear_to_page() helper to transfert
> skb header to part of page.
>
> We can reset the offset in the current sk->sk_sndmsg_page if we are the
> last user of the page.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
Applied.
^ permalink raw reply
* Re: [PATCH] drop_monitor: allow more events per second
From: David Miller @ 2012-04-21 20:29 UTC (permalink / raw)
To: eric.dumazet; +Cc: nhorman, netdev
In-Reply-To: <1334855781.2395.203.camel@edumazet-glaptop>
From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Thu, 19 Apr 2012 19:16:21 +0200
> From: Eric Dumazet <edumazet@google.com>
>
> It seems there is a logic error in trace_drop_common(), since we store
> only 64 drops, even if they are from same location.
>
> This fix is a one liner, but we probably need more work to avoid useless
> atomic dec/inc
>
> Now I can watch 1 Mpps drops through dropwatch...
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Neil Horman <nhorman@tuxdriver.com>
Applied, thanks.
^ permalink raw reply
* Re: [patch net-next 0/3] team: couple patches
From: David Miller @ 2012-04-21 20:27 UTC (permalink / raw)
To: jpirko; +Cc: netdev, eric.dumazet
In-Reply-To: <1334932926-23996-1-git-send-email-jpirko@redhat.com>
From: Jiri Pirko <jpirko@redhat.com>
Date: Fri, 20 Apr 2012 16:42:03 +0200
> Jiri Pirko (3):
> team: lb: let userspace care about port macs
> team: allow to enable/disable ports
> team: add per-port option for enabling/disabling ports
All applied, thanks Jiri.
^ 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