Date:   Sat, 18 Mar 2023 23:25:16 +0300
From:   Fedor Pchelkin <pchelkin@ispras.ru>
To:     Kalle Valo <kvalo@kernel.org>
Cc:     Toke =?utf-8?Q?H=C3=B8iland-J=C3=B8rgensen?= <toke@toke.dk>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Senthil Balasubramanian <senthilkumar@atheros.com>,
        "John W. Linville" <linville@tuxdriver.com>,
        Vasanthakumar Thiagarajan <vasanth@atheros.com>,
        Sujith <Sujith.Manoharan@atheros.com>,
        linux-wireless@vger.kernel.org, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        lvc-project@linuxtesting.org,
        syzbot+f2cb6e0ffdb961921e4d@syzkaller.appspotmail.com
Subject: Re: [PATCH 1/3] wifi: ath9k: avoid referencing uninit memory in
 ath9k_wmi_ctrl_rx
Message-ID: <20230318202516.2dpebysmo6uxilab@fpc>
References: <20230315202112.163012-1-pchelkin@ispras.ru>
 <20230315202112.163012-2-pchelkin@ispras.ru>
 <871qlovtho.fsf@kernel.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <871qlovtho.fsf@kernel.org>
Precedence: bulk

On Fri, Mar 17, 2023 at 07:26:11AM +0200, Kalle Valo wrote:
> 
> It would be also nice to know how you have tested these. Syzkaller is no
> substitute for testing on a real hardware.
> 

Unfortunately, currently I can't test this on real hardware so probably we
should postpone the patch discussion for some time. Roughly in a week or
two I'll be able to do some testing and try to reproduce the problem
there.

For sure this should be tested on real hardware as some issues may arise.
I sent the patch based on the commit b383e8abed41 ("wifi: ath9k: avoid
uninit memory read in ath9k_htc_rx_msg()") where it is explained
thoroughly what can lead to such behaviour. At the moment I don't see
anything in the code which can prevent that invalid scenario to happen for
endpoint callbacks path.

Actually, sanity checks for SKB length have been added everywhere inside
ath9k_htc_rx_msg() except where the endpoint callbacks are called. As for
the repro, the SKB inside ath9k_hif_usb_rx_stream() is allocated with
pkt_len=8 so it passes the 'htc_frame_hdr' check and processing in
ath9k_htc_rx_msg() but it obviously cannot be handled correctly in the
endpoint callbacks then.