netdev - Re: [Intel-wired-lan] [PATCH net v2 2/6] igc: Do not enable taprio offload for invalid arguments

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]

Message-ID: <f14f0d50-2569-0d47-91d6-23c1f99712ad@linux.intel.com>
Date: Mon, 3 Jul 2023 11:34:44 +0300
From: "naamax.meir" <naamax.meir@...ux.intel.com>
To: Florian Kauer <florian.kauer@...utronix.de>,
 Jesse Brandeburg <jesse.brandeburg@...el.com>,
 Tony Nguyen <anthony.l.nguyen@...el.com>,
 Vinicius Costa Gomes <vinicius.gomes@...el.com>,
 "David S . Miller" <davem@...emloft.net>, Eric Dumazet
 <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Tan Tee Min <tee.min.tan@...ux.intel.com>,
 Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>,
 Aravindhan Gunasekaran <aravindhan.gunasekaran@...el.com>,
 Malli C <mallikarjuna.chilakala@...el.com>
Cc: netdev@...r.kernel.org, kurt@...utronix.de,
 intel-wired-lan@...ts.osuosl.org, linux-kernel@...r.kernel.org
Subject: Re: [Intel-wired-lan] [PATCH net v2 2/6] igc: Do not enable taprio
 offload for invalid arguments

On 6/19/2023 13:08, Florian Kauer wrote:
> Only set adapter->taprio_offload_enable after validating the arguments.
> Otherwise, it stays set even if the offload was not enabled.
> Since the subsequent code does not get executed in case of invalid
> arguments, it will not be read at first.
> However, by activating and then deactivating another offload
> (e.g. ETF/TX launchtime offload), taprio_offload_enable is read
> and erroneously keeps the offload feature of the NIC enabled.
> 
> This can be reproduced as follows:
> 
>      # TAPRIO offload (flags == 0x2) and negative base-time leading to expected -ERANGE
>      sudo tc qdisc replace dev enp1s0 parent root handle 100 stab overhead 24 taprio \
> 	    num_tc 1 \
> 	    map 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 \
> 	    queues 1@0 \
> 	    base-time -1000 \
> 	    sched-entry S 01 300000 \
> 	    flags 0x2
> 
>      # IGC_TQAVCTRL is 0x0 as expected (iomem=relaxed for reading register)
>      sudo pcimem /sys/bus/pci/devices/0000:01:00.0/resource0 0x3570 w*1
> 
>      # Activate ETF offload
>      sudo tc qdisc replace dev enp1s0 parent root handle 6666 mqprio \
> 	    num_tc 3 \
> 	    map 2 2 1 0 2 2 2 2 2 2 2 2 2 2 2 2 \
> 	    queues 1@0 1@1 2@2 \
> 	    hw 0
>      sudo tc qdisc add dev enp1s0 parent 6666:1 etf \
> 	    clockid CLOCK_TAI \
> 	    delta 500000 \
> 	    offload
> 
>      # IGC_TQAVCTRL is 0x9 as expected
>      sudo pcimem /sys/bus/pci/devices/0000:01:00.0/resource0 0x3570 w*1
> 
>      # Deactivate ETF offload again
>      sudo tc qdisc delete dev enp1s0 parent 6666:1
> 
>      # IGC_TQAVCTRL should now be 0x0 again, but is observed as 0x9
>      sudo pcimem /sys/bus/pci/devices/0000:01:00.0/resource0 0x3570 w*1
> 
> Fixes: e17090eb2494 ("igc: allow BaseTime 0 enrollment for Qbv")
> Signed-off-by: Florian Kauer <florian.kauer@...utronix.de>
> Reviewed-by: Kurt Kanzenbach <kurt@...utronix.de>
> ---
>   drivers/net/ethernet/intel/igc/igc_main.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)

Tested-by: Naama Meir <naamax.meir@...ux.intel.com>