Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 11 Apr 2022 10:54:03 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Oleksij Rempel <linux@rempel-privat.de>
Cc:     Toke =?iso-8859-1?Q?H=F8iland-J=F8rgensen?= <toke@toke.dk>,
        Kalle Valo <kvalo@kernel.org>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        "John W. Linville" <linville@tuxdriver.com>,
        linux-wireless@vger.kernel.org, kernel-janitors@vger.kernel.org
Subject: Re: [PATCH] ath9k_htc: fix potential out of bounds access with
 invalid rxstatus->rs_keyix
Message-ID: <20220411075403.GZ12805@kadam>
References: <20220409061225.GA5447@kili>
 <00a0f9f7-2fbf-8883-38e7-edec3687e6a8@rempel-privat.de>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <00a0f9f7-2fbf-8883-38e7-edec3687e6a8@rempel-privat.de>
User-Agent: Mutt/1.9.4 (2018-02-28)
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?PPYiSyiTdLchwq95IaWj9WXOd4S/YigWSG3HcvdAygI8kXvIoJQFqeczd5vf?=
 =?us-ascii?Q?mM0IE6tlOfBXH9m4mtUK0SHB4cFiWaa26bo5wS9ZXuGzD6r1vd5vwJ4XApV3?=
 =?us-ascii?Q?u8x5TDtgjgQH1axKmro6D6ZmhUsHc08e9OlOTugkofOXVXfjaYd+lHbW9WsU?=
 =?us-ascii?Q?R/lKRjkckrdj+iQMcQ28wdZ2MmXo+Me2+U8PnjxGGb62IVSK2eopgGodTBof?=
 =?us-ascii?Q?KVlbauA+BqSOGGpRxmhORuCP2fsunU+NWlSlHRUzljHwebvOVPDMClVwhh+K?=
 =?us-ascii?Q?CwuyQ8a8Sh1A3I/Vnpt1g8TyMAO5/0aP8IPOEkPK/NteqFecSD7FNzzy9D0v?=
 =?us-ascii?Q?ZfFkqxcu/BQaO5aiyR97MEMTHuuZA1DjmtCANK6gh7FvahvHfd0wnQ9ZcRp2?=
 =?us-ascii?Q?7Xk8NARW2ydUKbGL815IS+6V9U53KQJs22BLlG1swMnRUGEIMeG1o4fsvSYk?=
 =?us-ascii?Q?G392Ax2yTQwmMBu0pwu0bcp64diOBlaiN6xsjpC4Ot6tPLQ9cyDvHnY12Nyr?=
 =?us-ascii?Q?ihJ8HK/nAwGDxwnPqL/2U92pegAFV7k92yRaQLJDXoiliG6q90pL3kOQw5OQ?=
 =?us-ascii?Q?XHfUB4ohMtmjxLelpCjlxQ5uJNYR33QHmz7eN/b11gHnbw2q5RKazd+6EhqO?=
 =?us-ascii?Q?Z2hKWvsH+uYJBKkmdQ+tqSpXg8td09f6hdObmlQXh02/7nWUDGsmCDgq0Twm?=
 =?us-ascii?Q?Pn42FEHMWmFDjVVrmoJSlmg83q4/d0fOLNUzfYJsrRTKiQ2z1M+ZXdwPl5x7?=
 =?us-ascii?Q?UWDXPXuJNgAwr2Dj1msgUj9nMdFC8Ihs4g9qXDjK2umd7txzaZ8P+dyOIa81?=
 =?us-ascii?Q?CDEmw0TiUVkFWT240csOF0IIzm2nLEW/i4eQaF5fyi0wAz/3VqTV3IC1K8N+?=
 =?us-ascii?Q?ozGSFNIj+JsKCKR6fM9QWBFU3UyZwtBKQU/0oZrZNEpCWYf+Kht8o4ptVMnU?=
 =?us-ascii?Q?vONZkpPoxr19N7cDIZpMu06xfTayj2jKe9UeSUSP3GIaTwaQK9K3v1QagbUk?=
 =?us-ascii?Q?zrhjov6UvgwtETwOa0PmQA3/B4ML508xKacMxX5oY+Mna5UK/ekf9PFtnf6j?=
 =?us-ascii?Q?PtMurZURFPw5h/wsAN6sUqx+EPUCVxwXe47HFsoOdNZFo6oP2gfW5qe3WcyM?=
 =?us-ascii?Q?sXnxX8IT+S5NDc8RpWlsdZQl//xRrSLMbhYahBn439E5ez/Yfj60iChIIVsi?=
 =?us-ascii?Q?gDZH+hpFqlCNncBoGXCRXJNbOHc4r63VjHWzZIUS5ElT07bg1Db3y5Gx/qGn?=
 =?us-ascii?Q?8zJK8/KxNLgQQiA1hI46tMwfAeNxceZrJWCAq2MRY7s9rM7T56WvGCunwBrk?=
 =?us-ascii?Q?9co7cZ30SUDTcc7qle5zXXX2kYcrfP26VzjY8DO4zL39DlRXJd2HsTJgGiol?=
 =?us-ascii?Q?gjtUXGL7AhflytIcvpcGxY7ycl8XQZvrJBlVGEvbU0PKW3FobhdpdagEO8co?=
 =?us-ascii?Q?7pARRmCnL61XRbzH9Bqs3ZNimmdilQu+Ax0BYSflWCUgiu1KqK/rWLyS4Txm?=
 =?us-ascii?Q?YKIO0X39OebefWWCp+Wu4T7Twsf7vS56Gf+NkSXcFi80mAHyyXHDW36xCIXC?=
 =?us-ascii?Q?CcuLscQ0iHi2ql11hx4vjcwAsvAwKk/D3EG7ZZ+sMEIcARaYXD8CL948C/NV?=
 =?us-ascii?Q?SrfMjNz2tJZu7IkOLBlAG6MYPqPkg1DjHuY6eXX3QN+JL9AHl6csRGsmo7m4?=
 =?us-ascii?Q?f+60kYLLxiib0AAMcG/+9Ynwvebfdsu4G4T8e/gSn0FYYu1uXErEwjhnMgCz?=
 =?us-ascii?Q?NH/acFI4sUeEOZYUwG4X6N0jL5odsA8=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 26346039-cb2e-4adc-e4ed-08da1b907feb
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 11 Apr 2022 07:54:26.6908
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: bppKqMOgBUKRpAgB7v1chXqZVXQb3SdAMR4BbYHDa/4pQim181luIGONvxHyPN1sSg/O7+Y/72IPPNVawlGrg7vZdLb++EWocuonaH6ntAs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR10MB5478
X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10434:6.0.425,18.0.858
 definitions=2022-04-11_02:2022-04-08,2022-04-11 signatures=0
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0 mlxscore=0
 bulkscore=0 phishscore=0 spamscore=0 mlxlogscore=999 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2202240000
 definitions=main-2204110043
X-Proofpoint-GUID: bxROymkyYl4m5k3VOq--RwUS3sKaJ2wM
X-Proofpoint-ORIG-GUID: bxROymkyYl4m5k3VOq--RwUS3sKaJ2wM
X-Spam-Status: No, score=-3.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_LOW,
        RCVD_IN_MSPIKE_H5,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

On Sat, Apr 09, 2022 at 09:53:53AM +0200, Oleksij Rempel wrote:
> > diff --git a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> > index 6a850a0bfa8a..a23eaca0326d 100644
> > --- a/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> > +++ b/drivers/net/wireless/ath/ath9k/htc_drv_txrx.c
> > @@ -1016,6 +1016,14 @@ static bool ath9k_rx_prepare(struct ath9k_htc_priv *priv,
> >   		goto rx_next;
> >   	}
> > 
> > +	if (rxstatus->rs_keyix >= ATH_KEYMAX &&
> > +	    rxstatus->rs_keyix != ATH9K_RXKEYIX_INVALID) {
> > +		ath_dbg(common, ANY,
> > +			"Invalid keyix, dropping (keyix: %d)\n",
> > +			rxstatus->rs_keyix);
> > +		goto rx_next;
> > +	}
> > +
> >   	/* Get the RX status information */
> > 
> >   	memset(rx_status, 0, sizeof(struct ieee80211_rx_status));
> 
> Looks ok to me.

Thanks!

> 
> By the way, rs_keyix seems to have a wrong type. It is declared as u8, but used as s8.

That sounds like something outside the scope of the patch...
Why do you mean "used as s8"?  Which function are you talking about?

You made me panic briefly because ATH9K_RXKEYIX_INVALID is a u8 so it
needs to be u8.  I would have thought instinctively that u8 would be the
right type for an index like this.

regards,
dan carpenter