* Re: [PATCH] r6040: use module_pci_driver macro
From: David Miller @ 2012-07-09 21:43 UTC (permalink / raw)
To: florian; +Cc: devendra.aaru, netdev
In-Reply-To: <201207082339.17996.florian@openwrt.org>
From: Florian Fainelli <florian@openwrt.org>
Date: Sun, 8 Jul 2012 23:39:17 +0200
> Le dimanche 08 juillet 2012 17:57:57, Devendra Naga a écrit :
>> as the manual of module_pci_driver says that
>> it can be used when the init and exit functions of
>> the module does nothing but the pci_register_driver
>> and pci_unregister_driver.
>>
>> use it for rdc's r6040 driver, as the init and exit
>> paths does as above, and also this reduces a little
>> amount of code.
>>
>> Signed-off-by: Devendra Naga <devendra.aaru@gmail.com>
>
> Acked-by: Florian Fainelli <florian@openwrt.org>
Applied.
^ permalink raw reply
* Re: [PATCH net-next] bnx2x: populate skb->l4_rxhash
From: David Miller @ 2012-07-09 21:41 UTC (permalink / raw)
To: eilong; +Cc: eric.dumazet, netdev, willemb
In-Reply-To: <1341850532.27035.1.camel@lb-tlvb-eilong.il.broadcom.com>
From: "Eilon Greenstein" <eilong@broadcom.com>
Date: Mon, 9 Jul 2012 19:15:32 +0300
> On Mon, 2012-07-09 at 18:02 +0200, Eric Dumazet wrote:
>> From: Eric Dumazet <edumazet@google.com>
>>
>> l4_rxhash is set on skb when rxhash is obtained from canonical 4-tuple
>> over transport ports/addresses.
>>
>> We can set skb->l4_rxhash for all incoming TCP packets on bnx2x for
>> free, as cqe status contains a hash type information.
>>
>> Signed-off-by: Eric Dumazet <edumazet@google.com>
...
> Acked-by: Eilon Greenstein <eilong@broadcom.com>
Applied, thanks.
^ permalink raw reply
* Re: [PATCH] fix RTPROT_RA markup of some RA routes in netlink
From: David Miller @ 2012-07-09 21:40 UTC (permalink / raw)
To: infrastation; +Cc: netdev
In-Reply-To: <1405671341819473@web18d.yandex.ru>
Please read Documentation/SubmittingPatches to learn how to submit
a proper patch with proper commit log message and a proper signoff.
The commit message should exactly describe the reason for your change
and how the change is implemented. It should not contain things like
"Hello" or "This is a patch" or "Thank you" or other irrelevant
verbiage that does not belong in the commit log message.
Thank you.
^ permalink raw reply
* Re: [PATCH] gianfar: fix potential sk_wmem_alloc imbalance
From: Paul Gortmaker @ 2012-07-09 21:38 UTC (permalink / raw)
To: Eric Dumazet
Cc: David Miller, netdev, Manfred Rudigier, Claudiu Manoil, Jiajun Wu,
Andy Fleming
In-Reply-To: <1341524713.3265.41.camel@edumazet-glaptop>
On 12-07-05 05:45 PM, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
>
> commit db83d136d7f753 (gianfar: Fix missing sock reference when
> processing TX time stamps) added a potential sk_wmem_alloc imbalance
>
> If the new skb has a different truesize than old one, we can get a
> negative sk_wmem_alloc once new skb is orphaned at TX completion.
>
> Now we no longer early orphan skbs in dev_hard_start_xmit(), this
> probably can lead to fatal bugs.
>
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Manfred Rudigier <manfred.rudigier@omicron.at>
> Cc: Claudiu Manoil <claudiu.manoil@freescale.com>
> Cc: Jiajun Wu <b06378@freescale.com>
> Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
> Cc: Andy Fleming <afleming@freescale.com>
> ---
>
> Note : I don't have the hardware and discovered this problem by code
> analysis. So please compile and run this patch before Acking it,
> thanks !
Compiles clean and boot tested with NFS rootfs on mpc8315erdb board
defconfig on net-next [5c9df5fed19 "small cleanup in ax25_addr_parse"]
>
> BTW, dev->needed_headroom should be set to GMAC_FCB_LEN + GMAC_TXPAL_LEN
> to avoid reallocations...
I also layered on the patch you sent for this and rebuilt, and it
too passes the same basic sanity test, so feel free to add a
Tested-by or similar to your headroom fix as well.
Paul.
>
> drivers/net/ethernet/freescale/gianfar.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/net/ethernet/freescale/gianfar.c b/drivers/net/ethernet/freescale/gianfar.c
> index f2db8fc..ab1d80f 100644
> --- a/drivers/net/ethernet/freescale/gianfar.c
> +++ b/drivers/net/ethernet/freescale/gianfar.c
> @@ -2063,10 +2063,9 @@ static int gfar_start_xmit(struct sk_buff *skb, struct net_device *dev)
> return NETDEV_TX_OK;
> }
>
> - /* Steal sock reference for processing TX time stamps */
> - swap(skb_new->sk, skb->sk);
> - swap(skb_new->destructor, skb->destructor);
> - kfree_skb(skb);
> + if (skb->sk)
> + skb_set_owner_w(skb_new, skb->sk);
> + consume_skb(skb);
> skb = skb_new;
> }
>
>
>
^ permalink raw reply
* Re: [PATCH 0/2] stmmac: nfs reboot crash & jumbo frame handling fix
From: David Miller @ 2012-07-09 21:38 UTC (permalink / raw)
To: deepak.sikri; +Cc: peppe.cavallaro, spear--sw-devel, netdev
In-Reply-To: <1341818086-28897-1-git-send-email-deepak.sikri@st.com>
From: Deepak Sikri <deepak.sikri@st.com>
Date: Mon, 9 Jul 2012 12:44:44 +0530
> This patch set handles in the fixes for following bugs that were
> observed during testing.
> 1. On Multiple reboot operations using nfs, system crash were observed
> with inconsistency in status of dma descriptors.
> 2. There were data losses observed whenever the jumbo frames were used
> for data transfers.
>
> Deepak Sikri (2):
> stmmac: Fix for nfs hang on multiple reboot
> stmmac: Fix for higher mtu size handling
All applied, thanks.
^ permalink raw reply
* Re: pull request: wireless 2012-07-09
From: David Miller @ 2012-07-09 21:29 UTC (permalink / raw)
To: linville; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20120709192818.GE1348@tuxdriver.com>
From: "John W. Linville" <linville@tuxdriver.com>
Date: Mon, 9 Jul 2012 15:28:18 -0400
> Please accept these fixes for the 3.5 stream...
Pulled, thanks John.
^ permalink raw reply
* Re: [RFC PATCH] net/ipv4/ipip: add support to move between network namespaces
From: David Miller @ 2012-07-09 21:19 UTC (permalink / raw)
To: christian.franke; +Cc: netdev
In-Reply-To: <1341848473-2666-1-git-send-email-christian.franke@adytonsystems.com>
You need to provide a proper signoff in your commit message. Yes I realize
this is an RFC, but better get your submission in correct format from the
beginning.
^ permalink raw reply
* Re: [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
From: Timur Tabi @ 2012-07-09 21:19 UTC (permalink / raw)
To: David Miller; +Cc: afleming, netdev
In-Reply-To: <20120709.141714.1072959067688919521.davem@davemloft.net>
David Miller wrote:
> Do you really mean that:
>
> foo = function(arg1,
> arg2);
>
> doesn't look like complete and utter shit to you?
It doesn't bother me, but I'll change it.
--
Timur Tabi
Linux kernel developer at Freescale
^ permalink raw reply
* Re: [PATCH] net/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
From: David Miller @ 2012-07-09 21:17 UTC (permalink / raw)
To: B04825; +Cc: afleming, netdev
In-Reply-To: <4FFAEB44.2030600@freescale.com>
From: Tabi Timur-B04825 <B04825@freescale.com>
Date: Mon, 9 Jul 2012 14:31:33 +0000
> David Miller wrote:
>
>> Define a macro for the timeout value rather than use an arbitrary
>> constant.
>
> Ok.
>
>>> + status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
>>> + 1000, 0);
>>
>> This indentation is absolutely terrible.
>
> Can you give me a clue as to how you think it should look? I could not
> come up with a good way to break up these lines and keep them under 80
> characters.
It's not the length of the first line, it's how the second line was
indented.
Always line up the arguments on the second and subsequent lines right
after the column containing the openning "(" on the first line.
foo = function(arg1,
arg2);
Do you really mean that:
foo = function(arg1,
arg2);
doesn't look like complete and utter shit to you?
^ permalink raw reply
* Re: New commands to configure IOV features
From: Chris Friesen @ 2012-07-09 21:13 UTC (permalink / raw)
To: Ben Hutchings; +Cc: Greg Rose, Yuval Mintz, netdev@vger.kernel.org
In-Reply-To: <1341859155.2535.43.camel@bwh-desktop.uk.solarflarecom.com>
On 07/09/2012 12:39 PM, Ben Hutchings wrote:
> On Tue, 2012-06-26 at 10:19 -0700, Greg Rose wrote:
>> On Tue, 26 Jun 2012 15:21:55 +0300
>> Yuval Mintz<yuvalmin@broadcom.com> wrote:
>>
>>> On 05/07/2012 06:16 PM, Greg Rose wrote:
>>>> The only way to configure the number of VFs continues to be through
>>>> the max_vfs module parameter. I've got a patch to do it through
>>>> ethtool sitting on the back burner but due to other requirements of
>>>> my day job I've not been able to work on it since last fall.
>>>>
>>>> - Greg
>>>
>>>
>>> Hi Ben,
>>>
>>> If I want to pick the RFCs and add support for configuring the number
>>> of VFs - do you think ethtool's the right place for such added
>>> support?
>>>
>>> I'm asking since as far as I can see, ethtool (today) doesn't contain
>>> any features related to virtual functions.
>>
>> I think a PCI utility tool would be better, SR-IOV is not limited to
>> network devices. That's one of the reasons I dropped the RFC. I
>> haven't gotten back to the idea since then due to my day job keeping me
>> pretty busy.
>
> For what it's worth, I agree with this.
I'd love to have the ability to set the number of VFs on the fly. From
my perspective it would be ideal if this could be exported via /sys or
something rather than a standalone tool using a binary API. It's fewer
packages for userspace to deal with as well as being easily scripted.
Chris
^ permalink raw reply
* Re: [PATCH net-next 1/2] r8169: support RTL8106E
From: David Miller @ 2012-07-09 21:10 UTC (permalink / raw)
To: romieu; +Cc: netdev, linux-kernel, hayeswang
In-Reply-To: <20120709101215.GA6530@electric-eye.fr.zoreil.com>
From: Francois Romieu <romieu@fr.zoreil.com>
Date: Mon, 9 Jul 2012 12:12:15 +0200
> David Miller <davem@davemloft.net> :
>> Francois, what would you like me to do with these two patches? I
>> haven't seen full ACKs from you yet.
>
> You have two options:
> - You can apply the first (8106) patch. I tested it. I'm ok with Hayes point.
> Consider it Acked.
> - You wait for a pull request with both patches + the incremental bits for
> the second one. Some of the incremental bits are needed in the original
> patch itself while some make more sense before the patch itself.
> ETA: this evening or tomorrow morning at worst.
>
> If you go for the first option, I'll simply skip the first patch and
> apply again. It's the same work for me.
I'll simply wait for your pull request, thank you.
^ permalink raw reply
* Re: [net-next patch v2] bnx2x: Add run-time CNIC support
From: David Miller @ 2012-07-09 21:10 UTC (permalink / raw)
To: meravs; +Cc: netdev, eilong, dmitry
In-Reply-To: <1341828055-4467-1-git-send-email-meravs@broadcom.com>
From: "Merav Sicron" <meravs@broadcom.com>
Date: Mon, 9 Jul 2012 13:00:55 +0300
> - max_non_def_sbs = bnx2x_get_num_non_def_sbs(pdev);
> + cnic_enabled = IS_ENABLED(CONFIG_CNIC) ? 1 : 0;
This, as I said, it pointless.
Every distribution is going to turn on this Kconfig option so guarding
this at all using the Kconfig option is largely valueless.
^ permalink raw reply
* [PATCH 2/2] bonding: debugfs and network namespaces are incompatible
From: Eric W. Biederman @ 2012-07-09 20:52 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Dilip Daya, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <87y5ms3bfi.fsf_-_-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
The bonding debugfs support has been broken in the presence of network
namespaces since it has been added. The debugfs support does not handle
multiple bonding devices with the same name in different network
namespaces.
I haven't had any bug reports, and I'm not interested in getting any.
Disable the debugfs support when network namespaces are enabled.
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
drivers/net/bonding/bond_debugfs.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/net/bonding/bond_debugfs.c b/drivers/net/bonding/bond_debugfs.c
index 3680aa2..2cf084e 100644
--- a/drivers/net/bonding/bond_debugfs.c
+++ b/drivers/net/bonding/bond_debugfs.c
@@ -6,7 +6,7 @@
#include "bonding.h"
#include "bond_alb.h"
-#ifdef CONFIG_DEBUG_FS
+#if defined(CONFIG_DEBUG_FS) && !defined(CONFIG_NET_NS)
#include <linux/debugfs.h>
#include <linux/seq_file.h>
--
1.7.5.4
^ permalink raw reply related
* [PATCH 1/2] bonding: Manage /proc/net/bonding/ entries from the netdev events
From: Eric W. Biederman @ 2012-07-09 20:51 UTC (permalink / raw)
To: David Miller
Cc: netdev-u79uwXL29TY76Z2rM5mHXA,
containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
Dilip Daya, linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20120706194741.GA22113-7LNsyQBKDXoIagZqoN9o3w@public.gmane.org>
It was recently reported that moving a bonding device between network
namespaces causes warnings from /proc. It turns out after the move we
were trying to add and to remove the /proc/net/bonding entries from the
wrong network namespace.
Move the bonding /proc registration code into the NETDEV_REGISTER and
NETDEV_UNREGISTER events where the proc registration and unregistration
will always happen at the right time.
Signed-off-by: "Eric W. Biederman" <ebiederm-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org>
---
drivers/net/bonding/bond_main.c | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 2ee8cf9..50de0fe 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -3226,6 +3226,12 @@ static int bond_master_netdev_event(unsigned long event,
switch (event) {
case NETDEV_CHANGENAME:
return bond_event_changename(event_bond);
+ case NETDEV_UNREGISTER:
+ bond_remove_proc_entry(event_bond);
+ break;
+ case NETDEV_REGISTER:
+ bond_create_proc_entry(event_bond);
+ break;
default:
break;
}
@@ -4410,8 +4416,6 @@ static void bond_uninit(struct net_device *bond_dev)
bond_work_cancel_all(bond);
- bond_remove_proc_entry(bond);
-
bond_debug_unregister(bond);
__hw_addr_flush(&bond->mc_list);
@@ -4813,7 +4817,6 @@ static int bond_init(struct net_device *bond_dev)
bond_set_lockdep_class(bond_dev);
- bond_create_proc_entry(bond);
list_add_tail(&bond->bond_list, &bn->dev_list);
bond_prepare_sysfs_group(bond);
--
1.7.5.4
^ permalink raw reply related
* Re: [PATCH 4/4] pch_gbe: vlan skb len fix
From: Ben Hutchings @ 2012-07-09 20:33 UTC (permalink / raw)
To: Andy Cress; +Cc: netdev
In-Reply-To: <40680C535D6FE6498883F1640FACD44D010BFC0E@ka-exchange-1.kontronamerica.local>
On Mon, 2012-07-09 at 06:30 -0700, Andy Cress wrote:
> Author: Veaceslav Falico <vfalico@redhat.com>
> Date: Tue Apr 10 08:14:17 2012 +0200
>
> pch_gbe: correctly verify skb->len in vlan case
> to avoid bogus transfer length errors.
This check shouldn't be necessary at all. Maybe you should remove it
instead of correcting the check?
Also, these patches are word-wrapped, and would not be applicable in
this form. See Documentation/email-clients.txt.
Ben.
> Signed-off-by: Andy Cress <andy.cress@us.kontron.com>
>
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> index 30ef285..04b0e49 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -2158,8 +2158,10 @@ static int pch_gbe_xmit_frame(struct sk_buff
> *skb, struct net_device *netdev)
> struct pch_gbe_adapter *adapter = netdev_priv(netdev);
> struct pch_gbe_tx_ring *tx_ring = adapter->tx_ring;
> unsigned long flags;
> + int offset;
>
> - if (unlikely(skb->len > (adapter->hw.mac.max_frame_size - 4))) {
> + offset = skb->protocol == htons(ETH_P_8021Q) ? 0 : 4;
> + if (unlikely(skb->len > (adapter->hw.mac.max_frame_size -
> offset))) {
> pr_err("Transfer length Error: skb len: %d > max: %d\n",
> skb->len, adapter->hw.mac.max_frame_size);
> dev_kfree_skb_any(skb);
--
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 1/4] pch_gbe: fix the checksum fill to the error location
From: Ben Hutchings @ 2012-07-09 20:16 UTC (permalink / raw)
To: Andy Cress; +Cc: netdev
In-Reply-To: <40680C535D6FE6498883F1640FACD44D010BFC0F@ka-exchange-1.kontronamerica.local>
On Mon, 2012-07-09 at 06:30 -0700, Andy Cress wrote:
> From: Zhong Hongbo <hongbo.zhong@windriver.com>
> Date: Mon, 9 Apr 2012 10:51:28 +0800
>
> Due to some unknown hardware limitations the pch_gbe hardware cannot
> calculate checksums when the length of network package is less
> than 64 bytes, where we will surprisingly encounter a problem of
> the destination IP incorrectly changed.
>
> When forwarding network packages at the network layer the IP packages
> won't be relayed to the upper transport layer and analyzed there,
> consequently, skb->transport_header pointer will be mistakenly remained
> the same as that of skb->network_header, resulting in TCP checksum
> wrongly
> filled into the field of destination IP in IP header.
>
> We can fix this issue by manually calculate the offset of the TCP
> checksum
> and update it accordingly.
[...]
You should be using skb_checksum_start_offset() instead.
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: [net-next RFC V5 5/5] virtio_net: support negotiating the number of queues through ctrl vq
From: Ben Hutchings @ 2012-07-09 20:13 UTC (permalink / raw)
To: Jason Wang
Cc: krkumar2, habanero, mashirle, kvm, mst, netdev, linux-kernel,
virtualization, edumazet, tahm, jwhan, davem, sri
In-Reply-To: <1341484194-8108-6-git-send-email-jasowang@redhat.com>
On Thu, 2012-07-05 at 18:29 +0800, Jason Wang wrote:
> This patch let the virtio_net driver can negotiate the number of queues it
> wishes to use through control virtqueue and export an ethtool interface to let
> use tweak it.
>
> As current multiqueue virtio-net implementation has optimizations on per-cpu
> virtuqueues, so only two modes were support:
>
> - single queue pair mode
> - multiple queue paris mode, the number of queues matches the number of vcpus
>
> The single queue mode were used by default currently due to regression of
> multiqueue mode in some test (especially in stream test).
>
> Since virtio core does not support paritially deleting virtqueues, so during
> mode switching the whole virtqueue were deleted and the driver would re-create
> the virtqueues it would used.
>
> btw. The queue number negotiating were defered to .ndo_open(), this is because
> only after feature negotitaion could we send the command to control virtqueue
> (as it may also use event index).
[...]
> +static int virtnet_set_channels(struct net_device *dev,
> + struct ethtool_channels *channels)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> + u16 queues = channels->rx_count;
> + unsigned status = VIRTIO_CONFIG_S_ACKNOWLEDGE | VIRTIO_CONFIG_S_DRIVER;
> +
> + if (channels->rx_count != channels->tx_count)
> + return -EINVAL;
[...]
> +static void virtnet_get_channels(struct net_device *dev,
> + struct ethtool_channels *channels)
> +{
> + struct virtnet_info *vi = netdev_priv(dev);
> +
> + channels->max_rx = vi->total_queue_pairs;
> + channels->max_tx = vi->total_queue_pairs;
> + channels->max_other = 0;
> + channels->max_combined = 0;
> + channels->rx_count = vi->num_queue_pairs;
> + channels->tx_count = vi->num_queue_pairs;
> + channels->other_count = 0;
> + channels->combined_count = 0;
> +}
[...]
It looks like the queue-pairs should be treated as 'combined channels',
not separate RX and TX channels. Also you don't need to clear the other
members; you can assume that the ethtool core will zero-initialise
structures for 'get' operations.
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/fsl_pq_mdio: use spin_event_timeout() to poll the indicator register
From: Jan Ceuleers @ 2012-07-09 19:50 UTC (permalink / raw)
To: Tabi Timur-B04825
Cc: David Miller, Fleming Andy-AFLEMING, netdev@vger.kernel.org
In-Reply-To: <4FFAEB44.2030600@freescale.com>
On 07/09/2012 04:31 PM, Tabi Timur-B04825 wrote:
>> This indentation is absolutely terrible.
>
> Can you give me a clue as to how you think it should look? I could not
> come up with a good way to break up these lines and keep them under 80
> characters.
>
How about this:
+ status = spin_event_timeout(!(in_be32(®s->miimind) & MIIMIND_BUSY),
+ 1000, 0);
On the second line use as many tabs as possible, then continue with spaces until the '1' in 1000 lines up with the '!'.
HTH, Jan
^ permalink raw reply
* Re: pull request: wireless 2012-07-09
From: John W. Linville @ 2012-07-09 19:43 UTC (permalink / raw)
To: davem; +Cc: linux-wireless, netdev, linux-kernel
In-Reply-To: <20120709192818.GE1348@tuxdriver.com>
[-- Attachment #1: Type: text/plain, Size: 8104 bytes --]
Forgot to sign -- hopefully this is sufficient? :-)
On Mon, Jul 09, 2012 at 03:28:18PM -0400, John W. Linville wrote:
> commit c1109736bcdd14b590392e9a96d66e7e823f9e67
>
> Dave,
>
> Please accept these fixes for the 3.5 stream...
>
> Bing Zhao provides an mwifiex fix for a memory leak uncovered by
> Coverity.
>
> Eliad Peller provides a mac80211 fix to properly clean-up after an
> association failure.
>
> Emmanuel Grumbach gives us an iwlegacy fix to avoid some on-device
> memory corruption that leads to a stuck queue.
>
> Sasha Levin offers an NFC fix to prevent a NULL pointer dereference
> in llcp_sock_getname.
>
> Stanislaw Gruszka offers an rt2x00usb fix for an RX queue stall that
> results from using the wrong argument order in a macro call. He also
> gives us an iwlegacy fix that corresponds to an already merged iwlwifi
> fix, the effect of which is to avoid stuck queue detection failure.
>
> Finally, Thomas Huehn provides a mac80211 fix to correct some kzalloc
> arguments. This was originally two patches, but I consolidated them.
>
> Please let me know if there are problems!
>
> Thanks,
>
> John
>
> ---
>
> The following changes since commit bb3bb3a5b839fa5c0a2c386640e865320a1fb64c:
>
> Merge branch 'master' of git://1984.lsi.us.es/nf (2012-07-09 02:47:59 -0700)
>
> are available in the git repository at:
>
>
> git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
>
> for you to fetch changes up to c1109736bcdd14b590392e9a96d66e7e823f9e67:
>
> Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless into for-davem (2012-07-09 15:09:08 -0400)
>
> ----------------------------------------------------------------
>
> Bing Zhao (1):
> mwifiex: fix Coverity SCAN CID 709078: Resource leak (RESOURCE_LEAK)
>
> Eliad Peller (1):
> mac80211: destroy assoc_data correctly if assoc fails
>
> Emmanuel Grumbach (1):
> iwlegacy: don't mess up the SCD when removing a key
>
> John W. Linville (1):
> Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem
>
> Sasha Levin (1):
> NFC: Prevent NULL deref when getting socket name
>
> Stanislaw Gruszka (2):
> rt2x00usb: fix indexes ordering on RX queue kick
> iwlegacy: always monitor for stuck queues
>
> Thomas Huehn (1):
> mac80211: correct size the argument to kzalloc in minstrel_ht
>
> drivers/net/wireless/iwlegacy/4965-mac.c | 4 ++--
> drivers/net/wireless/iwlegacy/common.c | 14 ++++++--------
> drivers/net/wireless/mwifiex/cfg80211.c | 1 +
> drivers/net/wireless/rt2x00/rt2x00usb.c | 2 +-
> net/mac80211/mlme.c | 6 ++----
> net/mac80211/rc80211_minstrel_ht.c | 2 +-
> net/nfc/llcp/sock.c | 2 +-
> 7 files changed, 14 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
> index 509301a..ff5d689 100644
> --- a/drivers/net/wireless/iwlegacy/4965-mac.c
> +++ b/drivers/net/wireless/iwlegacy/4965-mac.c
> @@ -3405,7 +3405,7 @@ il4965_remove_dynamic_key(struct il_priv *il,
> return 0;
> }
>
> - if (il->stations[sta_id].sta.key.key_offset == WEP_INVALID_OFFSET) {
> + if (il->stations[sta_id].sta.key.key_flags & STA_KEY_FLG_INVALID) {
> IL_WARN("Removing wrong key %d 0x%x\n", keyconf->keyidx,
> key_flags);
> spin_unlock_irqrestore(&il->sta_lock, flags);
> @@ -3420,7 +3420,7 @@ il4965_remove_dynamic_key(struct il_priv *il,
> memset(&il->stations[sta_id].sta.key, 0, sizeof(struct il4965_keyinfo));
> il->stations[sta_id].sta.key.key_flags =
> STA_KEY_FLG_NO_ENC | STA_KEY_FLG_INVALID;
> - il->stations[sta_id].sta.key.key_offset = WEP_INVALID_OFFSET;
> + il->stations[sta_id].sta.key.key_offset = keyconf->hw_key_idx;
> il->stations[sta_id].sta.sta.modify_mask = STA_MODIFY_KEY_MASK;
> il->stations[sta_id].sta.mode = STA_CONTROL_MODIFY_MSK;
>
> diff --git a/drivers/net/wireless/iwlegacy/common.c b/drivers/net/wireless/iwlegacy/common.c
> index cbf2dc1..5d4807c 100644
> --- a/drivers/net/wireless/iwlegacy/common.c
> +++ b/drivers/net/wireless/iwlegacy/common.c
> @@ -4767,14 +4767,12 @@ il_bg_watchdog(unsigned long data)
> return;
>
> /* monitor and check for other stuck queues */
> - if (il_is_any_associated(il)) {
> - for (cnt = 0; cnt < il->hw_params.max_txq_num; cnt++) {
> - /* skip as we already checked the command queue */
> - if (cnt == il->cmd_queue)
> - continue;
> - if (il_check_stuck_queue(il, cnt))
> - return;
> - }
> + for (cnt = 0; cnt < il->hw_params.max_txq_num; cnt++) {
> + /* skip as we already checked the command queue */
> + if (cnt == il->cmd_queue)
> + continue;
> + if (il_check_stuck_queue(il, cnt))
> + return;
> }
>
> mod_timer(&il->watchdog,
> diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
> index ce61b6f..5c7fd18 100644
> --- a/drivers/net/wireless/mwifiex/cfg80211.c
> +++ b/drivers/net/wireless/mwifiex/cfg80211.c
> @@ -958,6 +958,7 @@ static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
> case NL80211_HIDDEN_SSID_ZERO_CONTENTS:
> /* firmware doesn't support this type of hidden SSID */
> default:
> + kfree(bss_cfg);
> return -EINVAL;
> }
>
> diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
> index d357d1e..74ecc33 100644
> --- a/drivers/net/wireless/rt2x00/rt2x00usb.c
> +++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
> @@ -436,8 +436,8 @@ void rt2x00usb_kick_queue(struct data_queue *queue)
> case QID_RX:
> if (!rt2x00queue_full(queue))
> rt2x00queue_for_each_entry(queue,
> - Q_INDEX_DONE,
> Q_INDEX,
> + Q_INDEX_DONE,
> NULL,
> rt2x00usb_kick_rx_entry);
> break;
> diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
> index a4bb856..0db5d34 100644
> --- a/net/mac80211/mlme.c
> +++ b/net/mac80211/mlme.c
> @@ -2174,15 +2174,13 @@ ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
> sdata->name, mgmt->sa, status_code);
> ieee80211_destroy_assoc_data(sdata, false);
> } else {
> - printk(KERN_DEBUG "%s: associated\n", sdata->name);
> -
> if (!ieee80211_assoc_success(sdata, *bss, mgmt, len)) {
> /* oops -- internal error -- send timeout for now */
> - ieee80211_destroy_assoc_data(sdata, true);
> - sta_info_destroy_addr(sdata, mgmt->bssid);
> + ieee80211_destroy_assoc_data(sdata, false);
> cfg80211_put_bss(*bss);
> return RX_MGMT_CFG80211_ASSOC_TIMEOUT;
> }
> + printk(KERN_DEBUG "%s: associated\n", sdata->name);
>
> /*
> * destroy assoc_data afterwards, as otherwise an idle
> diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
> index 2d1acc6..f9e51ef 100644
> --- a/net/mac80211/rc80211_minstrel_ht.c
> +++ b/net/mac80211/rc80211_minstrel_ht.c
> @@ -809,7 +809,7 @@ minstrel_ht_alloc_sta(void *priv, struct ieee80211_sta *sta, gfp_t gfp)
> max_rates = sband->n_bitrates;
> }
>
> - msp = kzalloc(sizeof(struct minstrel_ht_sta), gfp);
> + msp = kzalloc(sizeof(*msp), gfp);
> if (!msp)
> return NULL;
>
> diff --git a/net/nfc/llcp/sock.c b/net/nfc/llcp/sock.c
> index 17a707d..e06d458 100644
> --- a/net/nfc/llcp/sock.c
> +++ b/net/nfc/llcp/sock.c
> @@ -292,7 +292,7 @@ static int llcp_sock_getname(struct socket *sock, struct sockaddr *addr,
>
> pr_debug("%p\n", sk);
>
> - if (llcp_sock == NULL)
> + if (llcp_sock == NULL || llcp_sock->dev == NULL)
> return -EBADFD;
>
> addr->sa_family = AF_NFC;
> --
> John W. Linville Someday the world will need a hero, and you
> linville@tuxdriver.com might be all we have. Be ready.
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]
^ permalink raw reply
* pull request: wireless 2012-07-09
From: John W. Linville @ 2012-07-09 19:28 UTC (permalink / raw)
To: davem-fT/PcQaiUtIeIZ0/mPfg9Q
Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
commit c1109736bcdd14b590392e9a96d66e7e823f9e67
Dave,
Please accept these fixes for the 3.5 stream...
Bing Zhao provides an mwifiex fix for a memory leak uncovered by
Coverity.
Eliad Peller provides a mac80211 fix to properly clean-up after an
association failure.
Emmanuel Grumbach gives us an iwlegacy fix to avoid some on-device
memory corruption that leads to a stuck queue.
Sasha Levin offers an NFC fix to prevent a NULL pointer dereference
in llcp_sock_getname.
Stanislaw Gruszka offers an rt2x00usb fix for an RX queue stall that
results from using the wrong argument order in a macro call. He also
gives us an iwlegacy fix that corresponds to an already merged iwlwifi
fix, the effect of which is to avoid stuck queue detection failure.
Finally, Thomas Huehn provides a mac80211 fix to correct some kzalloc
arguments. This was originally two patches, but I consolidated them.
Please let me know if there are problems!
Thanks,
John
---
The following changes since commit bb3bb3a5b839fa5c0a2c386640e865320a1fb64c:
Merge branch 'master' of git://1984.lsi.us.es/nf (2012-07-09 02:47:59 -0700)
are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless.git for-davem
for you to fetch changes up to c1109736bcdd14b590392e9a96d66e7e823f9e67:
Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/linville/wireless into for-davem (2012-07-09 15:09:08 -0400)
----------------------------------------------------------------
Bing Zhao (1):
mwifiex: fix Coverity SCAN CID 709078: Resource leak (RESOURCE_LEAK)
Eliad Peller (1):
mac80211: destroy assoc_data correctly if assoc fails
Emmanuel Grumbach (1):
iwlegacy: don't mess up the SCD when removing a key
John W. Linville (1):
Merge branch 'master' of git://git.kernel.org/.../linville/wireless into for-davem
Sasha Levin (1):
NFC: Prevent NULL deref when getting socket name
Stanislaw Gruszka (2):
rt2x00usb: fix indexes ordering on RX queue kick
iwlegacy: always monitor for stuck queues
Thomas Huehn (1):
mac80211: correct size the argument to kzalloc in minstrel_ht
drivers/net/wireless/iwlegacy/4965-mac.c | 4 ++--
drivers/net/wireless/iwlegacy/common.c | 14 ++++++--------
drivers/net/wireless/mwifiex/cfg80211.c | 1 +
drivers/net/wireless/rt2x00/rt2x00usb.c | 2 +-
net/mac80211/mlme.c | 6 ++----
net/mac80211/rc80211_minstrel_ht.c | 2 +-
net/nfc/llcp/sock.c | 2 +-
7 files changed, 14 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/iwlegacy/4965-mac.c b/drivers/net/wireless/iwlegacy/4965-mac.c
index 509301a..ff5d689 100644
--- a/drivers/net/wireless/iwlegacy/4965-mac.c
+++ b/drivers/net/wireless/iwlegacy/4965-mac.c
@@ -3405,7 +3405,7 @@ il4965_remove_dynamic_key(struct il_priv *il,
return 0;
}
- if (il->stations[sta_id].sta.key.key_offset == WEP_INVALID_OFFSET) {
+ if (il->stations[sta_id].sta.key.key_flags & STA_KEY_FLG_INVALID) {
IL_WARN("Removing wrong key %d 0x%x\n", keyconf->keyidx,
key_flags);
spin_unlock_irqrestore(&il->sta_lock, flags);
@@ -3420,7 +3420,7 @@ il4965_remove_dynamic_key(struct il_priv *il,
memset(&il->stations[sta_id].sta.key, 0, sizeof(struct il4965_keyinfo));
il->stations[sta_id].sta.key.key_flags =
STA_KEY_FLG_NO_ENC | STA_KEY_FLG_INVALID;
- il->stations[sta_id].sta.key.key_offset = WEP_INVALID_OFFSET;
+ il->stations[sta_id].sta.key.key_offset = keyconf->hw_key_idx;
il->stations[sta_id].sta.sta.modify_mask = STA_MODIFY_KEY_MASK;
il->stations[sta_id].sta.mode = STA_CONTROL_MODIFY_MSK;
diff --git a/drivers/net/wireless/iwlegacy/common.c b/drivers/net/wireless/iwlegacy/common.c
index cbf2dc1..5d4807c 100644
--- a/drivers/net/wireless/iwlegacy/common.c
+++ b/drivers/net/wireless/iwlegacy/common.c
@@ -4767,14 +4767,12 @@ il_bg_watchdog(unsigned long data)
return;
/* monitor and check for other stuck queues */
- if (il_is_any_associated(il)) {
- for (cnt = 0; cnt < il->hw_params.max_txq_num; cnt++) {
- /* skip as we already checked the command queue */
- if (cnt == il->cmd_queue)
- continue;
- if (il_check_stuck_queue(il, cnt))
- return;
- }
+ for (cnt = 0; cnt < il->hw_params.max_txq_num; cnt++) {
+ /* skip as we already checked the command queue */
+ if (cnt == il->cmd_queue)
+ continue;
+ if (il_check_stuck_queue(il, cnt))
+ return;
}
mod_timer(&il->watchdog,
diff --git a/drivers/net/wireless/mwifiex/cfg80211.c b/drivers/net/wireless/mwifiex/cfg80211.c
index ce61b6f..5c7fd18 100644
--- a/drivers/net/wireless/mwifiex/cfg80211.c
+++ b/drivers/net/wireless/mwifiex/cfg80211.c
@@ -958,6 +958,7 @@ static int mwifiex_cfg80211_start_ap(struct wiphy *wiphy,
case NL80211_HIDDEN_SSID_ZERO_CONTENTS:
/* firmware doesn't support this type of hidden SSID */
default:
+ kfree(bss_cfg);
return -EINVAL;
}
diff --git a/drivers/net/wireless/rt2x00/rt2x00usb.c b/drivers/net/wireless/rt2x00/rt2x00usb.c
index d357d1e..74ecc33 100644
--- a/drivers/net/wireless/rt2x00/rt2x00usb.c
+++ b/drivers/net/wireless/rt2x00/rt2x00usb.c
@@ -436,8 +436,8 @@ void rt2x00usb_kick_queue(struct data_queue *queue)
case QID_RX:
if (!rt2x00queue_full(queue))
rt2x00queue_for_each_entry(queue,
- Q_INDEX_DONE,
Q_INDEX,
+ Q_INDEX_DONE,
NULL,
rt2x00usb_kick_rx_entry);
break;
diff --git a/net/mac80211/mlme.c b/net/mac80211/mlme.c
index a4bb856..0db5d34 100644
--- a/net/mac80211/mlme.c
+++ b/net/mac80211/mlme.c
@@ -2174,15 +2174,13 @@ ieee80211_rx_mgmt_assoc_resp(struct ieee80211_sub_if_data *sdata,
sdata->name, mgmt->sa, status_code);
ieee80211_destroy_assoc_data(sdata, false);
} else {
- printk(KERN_DEBUG "%s: associated\n", sdata->name);
-
if (!ieee80211_assoc_success(sdata, *bss, mgmt, len)) {
/* oops -- internal error -- send timeout for now */
- ieee80211_destroy_assoc_data(sdata, true);
- sta_info_destroy_addr(sdata, mgmt->bssid);
+ ieee80211_destroy_assoc_data(sdata, false);
cfg80211_put_bss(*bss);
return RX_MGMT_CFG80211_ASSOC_TIMEOUT;
}
+ printk(KERN_DEBUG "%s: associated\n", sdata->name);
/*
* destroy assoc_data afterwards, as otherwise an idle
diff --git a/net/mac80211/rc80211_minstrel_ht.c b/net/mac80211/rc80211_minstrel_ht.c
index 2d1acc6..f9e51ef 100644
--- a/net/mac80211/rc80211_minstrel_ht.c
+++ b/net/mac80211/rc80211_minstrel_ht.c
@@ -809,7 +809,7 @@ minstrel_ht_alloc_sta(void *priv, struct ieee80211_sta *sta, gfp_t gfp)
max_rates = sband->n_bitrates;
}
- msp = kzalloc(sizeof(struct minstrel_ht_sta), gfp);
+ msp = kzalloc(sizeof(*msp), gfp);
if (!msp)
return NULL;
diff --git a/net/nfc/llcp/sock.c b/net/nfc/llcp/sock.c
index 17a707d..e06d458 100644
--- a/net/nfc/llcp/sock.c
+++ b/net/nfc/llcp/sock.c
@@ -292,7 +292,7 @@ static int llcp_sock_getname(struct socket *sock, struct sockaddr *addr,
pr_debug("%p\n", sk);
- if (llcp_sock == NULL)
+ if (llcp_sock == NULL || llcp_sock->dev == NULL)
return -EBADFD;
addr->sa_family = AF_NFC;
--
John W. Linville Someday the world will need a hero, and you
linville-2XuSBdqkA4R54TAoqtyWWQ@public.gmane.org might be all we have. Be ready.
--
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 11/16] netvm: Propagate page->pfmemalloc from skb_alloc_page to skb
From: Sebastian Andrzej Siewior @ 2012-07-09 19:18 UTC (permalink / raw)
To: Mel Gorman
Cc: Andrew Morton, Linux-MM, Linux-Netdev, LKML, David Miller,
Neil Brown, Peter Zijlstra, Mike Christie, Eric B Munson,
Eric Dumazet
In-Reply-To: <20120627084348.GG8271@suse.de>
On Wed, Jun 27, 2012 at 09:43:48AM +0100, Mel Gorman wrote:
> > You did not touch all drivers which use alloc_page(s)() like e1000(e). Was
> > this on purpose?
>
> Yes. The ones I changed were the semi-obvious ones and carried over from
> when the patches were completely out of tree. As the changelog notes
> it is not critical that these annotation happens and can be fixed on a
> per-driver basis if there are complains about network swapping being slow.
okay, I was just curious why some drivers were updated and others not.
> I can update e1000 if you like but it's not critical
> to do so and in fact getting a bug reporting saying that network swap
> was slow on e1000 would be useful to me in its own way :)
No, leave as it, I was just curious.
One thing: Do you think it makes sense to you introduce
#define GFP_NET_RX (GFP_ATOMIC | __GFP_MEMALLOC)
and use it within the receive path instead of GFP_ATOMIC?
Sebastian
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: pull request: wireless 2012-07-06
From: John W. Linville @ 2012-07-09 18:43 UTC (permalink / raw)
To: David Miller; +Cc: linux-wireless, netdev, linux-kernel, Thomas Huehn
In-Reply-To: <20120709.003155.284089706826738623.davem@davemloft.net>
On Mon, Jul 09, 2012 at 12:31:55AM -0700, David Miller wrote:
> From: "John W. Linville" <linville@tuxdriver.com>
> Date: Fri, 6 Jul 2012 15:20:35 -0400
>
> > Please let me know if there are problems!
>
> This indentation is not correct:
>
> commit 01f9cb073c827c60c43f769763b49a2026f1a897
> Author: Thomas Huehn <thomas@net.t-labs.tu-berlin.de>
> Date: Thu Jun 28 14:39:51 2012 -0700
>
> mwl8k: fix possible race condition in info->control.sta use
> ...
> + sta = ieee80211_find_sta_by_ifaddr(hw, wh->addr1,
> + wh->addr2);
OK, I'll drop this patch and send a new pull request with a couple more fixes as well.
John
--
John W. Linville Someday the world will need a hero, and you
linville@tuxdriver.com might be all we have. Be ready.
^ permalink raw reply
* Re: New commands to configure IOV features
From: Ben Hutchings @ 2012-07-09 18:39 UTC (permalink / raw)
To: Greg Rose; +Cc: Yuval Mintz, netdev@vger.kernel.org
In-Reply-To: <20120626101903.0000791c@unknown>
On Tue, 2012-06-26 at 10:19 -0700, Greg Rose wrote:
> On Tue, 26 Jun 2012 15:21:55 +0300
> Yuval Mintz <yuvalmin@broadcom.com> wrote:
>
> > On 05/07/2012 06:16 PM, Greg Rose wrote:
> >
> > > On Mon, 7 May 2012 14:17:54 +0300
> > > Yuval Mintz <yuvalmin@broadcom.com> wrote:
> > >
> > >> I've tried to figure out if there was a standard interface
> > >> (ethtool/iproute) through which a user could configure the number
> > >> of vfs in his system.
> > >>
> > >> I've seen the RFC suggested in
> > >> http://markmail.org/thread/qblfcv7zbxsxp7q6, and
> > >> http://markmail.org/thread/fw54dcppmxuxoe6n, but failed to see any
> > >> later references to it (commits or further discussion on this
> > >> topic).
> > >>
> > >> How exactly are things standing with these RFCs? Were they
> > >> abandoned?
> > >
> > > The only way to configure the number of VFs continues to be through
> > > the max_vfs module parameter. I've got a patch to do it through
> > > ethtool sitting on the back burner but due to other requirements of
> > > my day job I've not been able to work on it since last fall.
> > >
> > > - Greg
> >
> >
> > Hi Ben,
> >
> > If I want to pick the RFCs and add support for configuring the number
> > of VFs - do you think ethtool's the right place for such added
> > support?
> >
> > I'm asking since as far as I can see, ethtool (today) doesn't contain
> > any features related to virtual functions.
>
> I think a PCI utility tool would be better, SR-IOV is not limited to
> network devices. That's one of the reasons I dropped the RFC. I
> haven't gotten back to the idea since then due to my day job keeping me
> pretty busy.
For what it's worth, I agree with this.
Ben.
--
Ben Hutchings, Staff Engineer, Solarflare
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.
^ permalink raw reply
* Re: [PATCH net-next 05/15] security: selinux: Add AF_BUS socket SELinux hooks
From: Paul Moore @ 2012-07-09 18:38 UTC (permalink / raw)
To: Vincent Sanders
Cc: netdev, linux-kernel, David S. Miller, Javier Martinez Canillas
In-Reply-To: <1340988354-26981-6-git-send-email-vincent.sanders@collabora.co.uk>
On Friday, June 29, 2012 05:45:44 PM Vincent Sanders wrote:
> From: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>
> Add Security-Enhanced Linux (SELinux) hook for AF_BUS socket address family.
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Signed-off-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
It would be very helpful to include a description of how the access controls
would work.
>From looking at the other patches, it would appear that when a new socket
tries to connect to the AF_BUS bus it is checked against the security label of
the bus master, yes? Further, if no bus master is present, the connect() is
denied at the AF_BUS level in the bus_connect() function, yes?
Have you considered the socket_getpeersec_dgram() hook? Since AF_BUS does not
appear to be stream oriented I think you can safely ignore
socket_getpeersec_stream().
Have you considered the unix_may_send() hook? Ignoring the AF_UNIX specific
name, it seems like a reasonable hook for AF_BUS; unless you don't expect to
have any read-only AF_BUS clients in which case the connect() hook should be
enough (it would implicitly grant read/write access to each socket in that
case).
Finally, as others have said, you need to ensure that you CC the LSM and
SELinux lists on the relevant patches as well as provide LSM hook
implementations for LSMs other than SELinux where it makes sense (not all LSMs
will require implementations for the new hooks).
> ---
> security/selinux/hooks.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 4ee6f23..5bacbe2 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -67,6 +67,7 @@
> #include <linux/quota.h>
> #include <linux/un.h> /* for Unix socket types */
> #include <net/af_unix.h> /* for Unix socket types */
> +#include <net/af_bus.h> /* for Bus socket types */
> #include <linux/parser.h>
> #include <linux/nfs_mount.h>
> #include <net/ipv6.h>
> @@ -4101,6 +4102,39 @@ static int selinux_socket_unix_may_send(struct socket
> *sock, &ad);
> }
>
> +static int selinux_socket_bus_connect(struct sock *sock, struct sock
> *other, + struct sock *newsk)
> +{
> + struct sk_security_struct *sksec_sock = sock->sk_security;
> + struct sk_security_struct *sksec_other = other->sk_security;
> + struct sk_security_struct *sksec_new = newsk->sk_security;
> + struct common_audit_data ad;
> + struct lsm_network_audit net = {0,};
> + int err;
> +
> + ad.type = LSM_AUDIT_DATA_NET;
> + ad.u.net = &net;
> + ad.u.net->sk = other;
> +
> + err = avc_has_perm(sksec_sock->sid, sksec_other->sid,
> + sksec_other->sclass,
> + UNIX_STREAM_SOCKET__CONNECTTO, &ad);
See my earlier comments about the similarities between this new hook and the
existing AF_UNIX hook. The fact that you are reusing the
UNIX_STREAM_SOCKET__CONNECTTO permission (which is likely a no-no BTW) only
reinforces the similarities between the two.
> + if (err)
> + return err;
> +
> + /* server child socket */
> + sksec_new->peer_sid = sksec_sock->sid;
> + err = security_sid_mls_copy(sksec_other->sid, sksec_sock->sid,
> + &sksec_new->sid);
> + if (err)
> + return err;
> +
> + /* connecting socket */
> + sksec_sock->peer_sid = sksec_new->sid;
> +
> + return 0;
> +}
> +
> static int selinux_inet_sys_rcv_skb(int ifindex, char *addrp, u16 family,
> u32 peer_sid,
> struct common_audit_data *ad)
> @@ -5643,6 +5677,7 @@ static struct security_operations selinux_ops = {
>
> .unix_stream_connect = selinux_socket_unix_stream_connect,
> .unix_may_send = selinux_socket_unix_may_send,
> + .bus_connect = selinux_socket_bus_connect,
>
> .socket_create = selinux_socket_create,
> .socket_post_create = selinux_socket_post_create,
--
paul moore
www.paul-moore.com
^ permalink raw reply
* Re: [PATCH net-next 04/15] security: Add Linux Security Modules hook for AF_BUS sockets
From: Paul Moore @ 2012-07-09 18:02 UTC (permalink / raw)
To: Vincent Sanders
Cc: netdev, linux-kernel, David S. Miller, Javier Martinez Canillas,
casey
In-Reply-To: <1340988354-26981-5-git-send-email-vincent.sanders@collabora.co.uk>
On Friday, June 29, 2012 05:45:43 PM Vincent Sanders wrote:
> From: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>
> AF_BUS implements a security hook bus_connect() to be used by LSM to
> enforce connectivity security policies.
>
> Signed-off-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
> Signed-off-by: Vincent Sanders <vincent.sanders@collabora.co.uk>
In future postings, please reorder the patchset so that this patch (and the
LSM specific patches) are applied after the actual AF_BUS implementation
(patch 08/15 in this patchset). This makes it easier to quickly understand
how the LSM hooks/implementation interacts with the AF_BUS code.
A good rule of thumb that I try to follow when submitting large patchsets is
that each patch should contain code that won't be optimized away during the
build because there is no caller. Sometimes that isn't possible without
making things overly awkward, but in this particular case it shouldn't cause a
problem.
> ---
> include/linux/security.h | 11 +++++++++++
> security/capability.c | 7 +++++++
> security/security.c | 7 +++++++
> 3 files changed, 25 insertions(+)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 4e5a73c..d30dc4a 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
...
> +static inline int security_bus_connect(struct socket *sock,
> + struct sock *other,
> + struct sock *newsk)
> +{
> + return 0;
> +}
> +
Other than the AF_UNIX specific name, is there a reason why you chose not to
reuse the unix_stream_connect() LSM hook? The arguments are the same, and
based on an initial quick review of the SELinux hook implementations they
appear to do almost identical things; the permissions are different but it
should be trivial to make that conditional on the parent socket's address
family (SELinux does similar things with other socket operations). Looking at
the Smack implementation, I don't think it would be a problem there either
(CC'd Casey for his thoughts).
I'm still reviewing the rest of the AF_BUS patches but wanted to ask this now
in case I was missing something.
--
paul moore
www.paul-moore.com
^ 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