netdev - Re: [PATCH] mlx4: Fix tx ring affinity

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 28 Apr 2015 16:37:33 +0300
From:	Ido Shamay <idos@....mellanox.co.il>
To:	Benjamin Poirier <benjamin.poirier@...il.com>
CC:	Amir Vadai <amirv@...lanox.com>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org, Ido Shamay <idos@...lanox.com>
Subject: Re: [PATCH] mlx4: Fix tx ring affinity_mask creation

On 4/28/2015 6:26 AM, Benjamin Poirier wrote:
> On 2015/04/13 17:22, Benjamin Poirier wrote:
>> On 2015/04/12 10:03, Ido Shamay wrote:
>>> Hi Benjamin,
>>>
>>> On 4/10/2015 7:27 PM, Benjamin Poirier wrote:
>>>> By default, the number of tx queues is limited by the number of online cpus in
>>>> mlx4_en_get_profile(). However, this limit no longer holds after the ethtool
>>>> .set_channels method has been called. In that situation, the driver may access
>>>> invalid bits of certain cpumask variables when queue_index > nr_cpu_ids.
>>> I must say I don't see the above issue with the current code.
>>> Whatever is the modified value of priv->num_tx_rings_p_up, it will set XPS
>>> only on queues which have
>>> been set with CPU affinity mask (no access to invalid bits).
>> The problem is not with the call to netif_set_xps_queue() it is with the
>> calls to cpu_online() and cpumask_set_cpu().
>>
>> For example, if the user calls `ethtool -L ethX tx 32`, queue_index in
>> mlx4_en_create_tx_ring() can be up to 255. Depending on CONFIG_NR_CPUS
>> and CONFIG_CPUMASK_OFFSTACK this may result in calls to cpu_online() and
>> cpumask_set_cpu() with cpu >= nr_cpumask_bits which is an invalid usage
>> of the cpumask api. The driver will potentially read or write beyond the
>> end of the bitmap. With CONFIG_CPUMASK_OFFSTACK=y and
>> CONFIG_DEBUG_PER_CPU_MAPS=y, the aforementioned ethtool call on a system
>> with <32 cpus triggers the warning in cpumask_check().
>>
> Mellanox, can you please
> ack the patch as submitted, or
> clarify what changes you'd like to see given my reply above, or
> submit a fix of your own for this problem
>
> Thanks,
> -Benjamin
Hi Benjamin,

After further review and better understanding of the issue, we are okay 
with your patch as is.
Thanks for the good work.

Acked-by: Ido Shamay <idos@...lanox.com>

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html