This patch fixes a Kernel Panic in bonding driver debugfs file: rlb_hash_table.
$> modprobe bonding mode=6
$> cat /sys/kernel/debug/bonding/bond0/rlb_hash_table
This will crash the kernel. The struct alb_bond_info is initialized only when
the bonding interface is initialized (ip link set bond0 up) and not at the time
it is allocated. If we try to read the table before that, it'll result in a
kernel panic.
The patch applies against both net and net-next
Signed-off-by: Vishwanath Pai <[email protected]>
diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
index 089a402..806892a 100644
--- a/drivers/net/bonding/bond_main.c
+++ b/drivers/net/bonding/bond_main.c
@@ -4510,6 +4510,8 @@ unsigned int bond_get_num_tx_queues(void)
int bond_create(struct net *net, const char *name)
{
struct net_device *bond_dev;
+ struct bonding *bond;
+ struct alb_bond_info *bond_info;
int res;
rtnl_lock();
@@ -4523,6 +4525,14 @@ int bond_create(struct net *net, const char *name)
return -ENOMEM;
}
+ /*
+ * Initialize rx_hashtbl_used_head to RLB_NULL_INDEX.
+ * It is set to 0 by default which is wrong.
+ */
+ bond = netdev_priv(bond_dev);
+ bond_info = &(BOND_ALB_INFO(bond));
+ bond_info->rx_hashtbl_used_head = RLB_NULL_INDEX;
+
dev_net_set(bond_dev, net);
bond_dev->rtnl_link_ops = &bond_link_ops;
On Wed, Apr 29, 2015 at 02:24:23PM -0400, Pai wrote:
> This patch fixes a Kernel Panic in bonding driver debugfs file: rlb_hash_table.
>
> $> modprobe bonding mode=6
> $> cat /sys/kernel/debug/bonding/bond0/rlb_hash_table
>
> This will crash the kernel. The struct alb_bond_info is initialized only when
> the bonding interface is initialized (ip link set bond0 up) and not at the time
> it is allocated. If we try to read the table before that, it'll result in a
> kernel panic.
Nice catch!
>
> The patch applies against both net and net-next
>
> Signed-off-by: Vishwanath Pai <[email protected]>
Signed-off-by: Andy Gospodarek <[email protected]>
>
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 089a402..806892a 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4510,6 +4510,8 @@ unsigned int bond_get_num_tx_queues(void)
> int bond_create(struct net *net, const char *name)
> {
> struct net_device *bond_dev;
> + struct bonding *bond;
> + struct alb_bond_info *bond_info;
> int res;
>
> rtnl_lock();
> @@ -4523,6 +4525,14 @@ int bond_create(struct net *net, const char *name)
> return -ENOMEM;
> }
>
> + /*
> + * Initialize rx_hashtbl_used_head to RLB_NULL_INDEX.
> + * It is set to 0 by default which is wrong.
> + */
> + bond = netdev_priv(bond_dev);
> + bond_info = &(BOND_ALB_INFO(bond));
> + bond_info->rx_hashtbl_used_head = RLB_NULL_INDEX;
> +
> dev_net_set(bond_dev, net);
> bond_dev->rtnl_link_ops = &bond_link_ops;
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
From: Andy Gospodarek <[email protected]>
Date: Wed, 29 Apr 2015 14:51:07 -0400
> On Wed, Apr 29, 2015 at 02:24:23PM -0400, Pai wrote:
>> This patch fixes a Kernel Panic in bonding driver debugfs file: rlb_hash_table.
>>
>> $> modprobe bonding mode=6
>> $> cat /sys/kernel/debug/bonding/bond0/rlb_hash_table
>>
>> This will crash the kernel. The struct alb_bond_info is initialized only when
>> the bonding interface is initialized (ip link set bond0 up) and not at the time
>> it is allocated. If we try to read the table before that, it'll result in a
>> kernel panic.
>
> Nice catch!
>
>>
>> The patch applies against both net and net-next
>>
>> Signed-off-by: Vishwanath Pai <[email protected]>
>
> Signed-off-by: Andy Gospodarek <[email protected]>
Applied, thanks.
On 04/29/2015 08:24 PM, Pai wrote:
> This patch fixes a Kernel Panic in bonding driver debugfs file: rlb_hash_table.
>
> $> modprobe bonding mode=6
> $> cat /sys/kernel/debug/bonding/bond0/rlb_hash_table
>
> This will crash the kernel. The struct alb_bond_info is initialized only when
> the bonding interface is initialized (ip link set bond0 up) and not at the time
> it is allocated. If we try to read the table before that, it'll result in a
> kernel panic.
>
> The patch applies against both net and net-next
>
> Signed-off-by: Vishwanath Pai <[email protected]>
>
Good catch, a few cosmetic nits below.
> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
> index 089a402..806892a 100644
> --- a/drivers/net/bonding/bond_main.c
> +++ b/drivers/net/bonding/bond_main.c
> @@ -4510,6 +4510,8 @@ unsigned int bond_get_num_tx_queues(void)
> int bond_create(struct net *net, const char *name)
> {
> struct net_device *bond_dev;
> + struct bonding *bond;
> + struct alb_bond_info *bond_info;
Please arrange these longest to shortest line, it's easier to read.
> int res;
>
> rtnl_lock();
> @@ -4523,6 +4525,14 @@ int bond_create(struct net *net, const char *name)
> return -ENOMEM;
> }
>
> + /*
> + * Initialize rx_hashtbl_used_head to RLB_NULL_INDEX.
> + * It is set to 0 by default which is wrong.
> + */
Multiline comment style of networking content is:
/* text
* text
*/
See: Documentation/networking/netdev-FAQ.txt
Alternatively you can create an inline with descriptive name that does the
initialization and wouldn't need the comment. Either way's fine by me.
Cheers,
Nik
> + bond = netdev_priv(bond_dev);
> + bond_info = &(BOND_ALB_INFO(bond));
> + bond_info->rx_hashtbl_used_head = RLB_NULL_INDEX;
> +
> dev_net_set(bond_dev, net);
> bond_dev->rtnl_link_ops = &bond_link_ops;
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
On 04/29/2015 03:45 PM, Nikolay Aleksandrov wrote:
> On 04/29/2015 08:24 PM, Pai wrote:
>> This patch fixes a Kernel Panic in bonding driver debugfs file: rlb_hash_table.
>>
>> $> modprobe bonding mode=6
>> $> cat /sys/kernel/debug/bonding/bond0/rlb_hash_table
>>
>> This will crash the kernel. The struct alb_bond_info is initialized only when
>> the bonding interface is initialized (ip link set bond0 up) and not at the time
>> it is allocated. If we try to read the table before that, it'll result in a
>> kernel panic.
>>
>> The patch applies against both net and net-next
>>
>> Signed-off-by: Vishwanath Pai <[email protected]>
>>
> Good catch, a few cosmetic nits below.
>
>> diff --git a/drivers/net/bonding/bond_main.c b/drivers/net/bonding/bond_main.c
>> index 089a402..806892a 100644
>> --- a/drivers/net/bonding/bond_main.c
>> +++ b/drivers/net/bonding/bond_main.c
>> @@ -4510,6 +4510,8 @@ unsigned int bond_get_num_tx_queues(void)
>> int bond_create(struct net *net, const char *name)
>> {
>> struct net_device *bond_dev;
>> + struct bonding *bond;
>> + struct alb_bond_info *bond_info;
> Please arrange these longest to shortest line, it's easier to read.
>
>> int res;
>>
>> rtnl_lock();
>> @@ -4523,6 +4525,14 @@ int bond_create(struct net *net, const char *name)
>> return -ENOMEM;
>> }
>>
>> + /*
>> + * Initialize rx_hashtbl_used_head to RLB_NULL_INDEX.
>> + * It is set to 0 by default which is wrong.
>> + */
> Multiline comment style of networking content is:
> /* text
> * text
> */
>
> See: Documentation/networking/netdev-FAQ.txt
>
> Alternatively you can create an inline with descriptive name that does the
> initialization and wouldn't need the comment. Either way's fine by me.
>
> Cheers,
> Nik
>
>> + bond = netdev_priv(bond_dev);
>> + bond_info = &(BOND_ALB_INFO(bond));
>> + bond_info->rx_hashtbl_used_head = RLB_NULL_INDEX;
>> +
>> dev_net_set(bond_dev, net);
>> bond_dev->rtnl_link_ops = &bond_link_ops;
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe netdev" in
>> the body of a message to [email protected]
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
David,
Since the patch has been applied already - should I send a V2 with the
suggested changes? Or submit a new patch that applies on top of the
first patch?
Thanks,
Vishwanath
From: Vishwanath Pai <[email protected]>
Date: Wed, 29 Apr 2015 16:30:44 -0400
> Since the patch has been applied already - should I send a V2 with the
> suggested changes? Or submit a new patch that applies on top of the
> first patch?
You should always send me a relative patch in this situation.