Received-SPF: pass (google.com: domain of linux-wireless-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Date:   Wed, 3 Nov 2021 13:21:29 +0300
From:   Dan Carpenter <dan.carpenter@oracle.com>
To:     Pkshih <pkshih@realtek.com>
Cc:     Colin King <colin.king@canonical.com>,
        Kalle Valo <kvalo@codeaurora.org>,
        "David S . Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>,
        "linux-wireless@vger.kernel.org" <linux-wireless@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "kernel-janitors@vger.kernel.org" <kernel-janitors@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH][next] rtw89: Fix potential dereference of the null
 pointer sta
Message-ID: <20211103102128.GL2794@kadam>
References: <20211015154530.34356-1-colin.king@canonical.com>
 <9cc681c217a449519aee524b35e6b6bc@realtek.com>
 <20211102131437.GF2794@kadam>
 <c3de973999ea40cf967ffefe0ee56ed4@realtek.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <c3de973999ea40cf967ffefe0ee56ed4@realtek.com>
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?yyTNsEnUDpamse1PzF3MUZ4vFtBgoZZwbyinJCj+dvHmylWGLRyFVVAIzAi2?=
 =?us-ascii?Q?qCnR/1gF8xsNC0JSKGRn9VFjZS1hQ81aCB3k2TOgOHPRSfyduuIzWJx0b1Hq?=
 =?us-ascii?Q?+MWejd5966OnyCIpEY8DV6Msu3Q1RXP5AcOGDL/6dwfNEhj9sFCZSQXNJZmz?=
 =?us-ascii?Q?1SMt4yWoC6WxfkXBzXjr2T7rgmU8pMkrBSWW6LByJoBIyXldhUXuWpwAVGB4?=
 =?us-ascii?Q?YZteZehoLxrX7E59zYCUMojlyjgw0V2lr5RK2qj+Okhmcarn/vbfVnc6+vqM?=
 =?us-ascii?Q?JGKc5u8+UCk8VxyDjb4tNJw5dA/asj211vNVegqW9TLSlGVKBrTsRaAGCAOF?=
 =?us-ascii?Q?16tBc/E5u3USruXe2+2bF1YtsE0OspJJUvAIZa7YA6qc5awiM76SkWcRbB1W?=
 =?us-ascii?Q?d1YiJQU4rMANB5b4hdFAeyNNe6TCr6toYe8XgrDO7FqIkRYXA/nxR78FOBr3?=
 =?us-ascii?Q?4PLlJucIeQsU7TiUkkrunpc2oMfCgNHUpSNwnqNCrnusrlRhFbhIl5hS5vdV?=
 =?us-ascii?Q?rJGFX8muXnjbfmjtNPNmuedoeRF61N20tCvZe1lnjYA+BpYoCMvFT6j8QSxW?=
 =?us-ascii?Q?J7Vgc7AUxqWtA+cvyK9qA8JMvy2e/4h3ZeH3pvnQ1Ckr3fWh9jTNDQJgHjNT?=
 =?us-ascii?Q?zp43e6JtWupGVOH7M718nQAGPi+YF9RnEGuO/CqqS4aMLrNjkXCRw/uw5w1o?=
 =?us-ascii?Q?Pk/lCUqMiGJkAv78NNWtBGFm9jh9zXDDYeRmz6UHFWTiwYTqPCB5pNVmgQuo?=
 =?us-ascii?Q?CIccR0sHT8dQjL4XimVdf/WI/znouNlhcgQIB9G1MbLuKEXYY2r+m/RPd5dY?=
 =?us-ascii?Q?J/AiWWEjdB2GVhogRLNPMnMiHY3A0aCmuQIvwtMtlpXjLkH7UjXknppVaIUd?=
 =?us-ascii?Q?w04/WY6Yu/nT5/pIol6k4bvnEOeMOd+oK8gD9OL4nEq1lAhYEWINY9S0Wwxr?=
 =?us-ascii?Q?qfs8bTkESnkamSSCwTBwBIPvdXkTcVeC1tHX2AWfPB6pa1hKaOuUvfpl4Cr7?=
 =?us-ascii?Q?wliJQGJMN1jrqoPUrMf/0C1Yt8CGJU9THdAlGgedGXat+7wMLGvKBhIqlu2c?=
 =?us-ascii?Q?GUM54d8hmJjyi2KEKDokepFOFNXUq8MKZbzyElT1Q8WX5X6grbxYU+RQv4ng?=
 =?us-ascii?Q?/snjaqLZfwDMJxtqV22zclPDcprM1IS3OUeo3u46+mgRrH4NyTWdrbJbKFkj?=
 =?us-ascii?Q?1I3AecD2HZxYOzH3n4y0Qu/Q7DvnbyGtGWcpsnad4wM1oAv171bwbf/AtVOR?=
 =?us-ascii?Q?cp/+q5hyGCzgB83+VkTrB4IFkRufaRZtEyFXrVSAGQKWrpaIdZeXfMYa9o9t?=
 =?us-ascii?Q?fZkHiTBKYd7jfnRthbup6vFau56Ll6KS4VcueZW8+t7hn3W/w1B2KvJBtmg7?=
 =?us-ascii?Q?LdBClk7yZlT+jTCKS4jSABpCHpXXeXUjNRaXPyjhoHjcgmOOaGGOLgnHgJjd?=
 =?us-ascii?Q?RrLJ5Vx+83PsAUYAbko3d8q68nldISfH1VDGyg8PgpjwJxq30ruVQjVY85K7?=
 =?us-ascii?Q?rGa/a1AMyJgMjJ+Xu+rBxCCp98GNELSrxnrNGYXpqfVoY+4ExFplhxS47yNm?=
 =?us-ascii?Q?HRBFMM1/EZe2JpQw+4y1pHC00REzkOmBMciBgFp4fMpRg8pT3tYMobRVjGN5?=
 =?us-ascii?Q?jyhKKrvVk6Gl+d77mMKHSe60nrqIZe6YCApedaofTDlx+GrJTjEC2aBsnY+3?=
 =?us-ascii?Q?IZJmvw=3D=3D?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a4eb4fe1-05b5-432a-83e4-08d99eb3bdcc
X-MS-Exchange-CrossTenant-AuthSource: MWHPR1001MB2365.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 03 Nov 2021 10:21:47.6173
 (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: rNG68CLN3znkKhDO4DflMwqoxPGo6uwF5h8XunT5V8K4dtNgTcvV0MV7siXX5VSC6wWbfi60tsSsgDsJ+UDPMUDK9zM3zPJqN0qtDsBpZMY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MWHPR10MB1615
X-Proofpoint-Virus-Version: vendor=nai engine=6300 definitions=10156 signatures=668683
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxscore=0 spamscore=0
 malwarescore=0 mlxlogscore=999 adultscore=0 phishscore=0 bulkscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2110150000
 definitions=main-2111030058
X-Proofpoint-ORIG-GUID: lvMC1EOH15DkpoNSXZMlZYECHCHOjRXB
X-Proofpoint-GUID: lvMC1EOH15DkpoNSXZMlZYECHCHOjRXB
Precedence: bulk
List-ID: <linux-wireless.vger.kernel.org>
X-Mailing-List: linux-wireless@vger.kernel.org

On Wed, Nov 03, 2021 at 12:36:17AM +0000, Pkshih wrote:

> > > > diff --git a/drivers/net/wireless/realtek/rtw89/core.c
> > > > b/drivers/net/wireless/realtek/rtw89/core.c
> > > > index 06fb6e5b1b37..26f52a25f545 100644
> > > > --- a/drivers/net/wireless/realtek/rtw89/core.c
> > > > +++ b/drivers/net/wireless/realtek/rtw89/core.c
> > > > @@ -1534,9 +1534,14 @@ static bool rtw89_core_txq_agg_wait(struct rtw89_dev *rtwdev,
> > > >  {
> > > >  	struct rtw89_txq *rtwtxq = (struct rtw89_txq *)txq->drv_priv;
> > > >  	struct ieee80211_sta *sta = txq->sta;
> > > > -	struct rtw89_sta *rtwsta = (struct rtw89_sta *)sta->drv_priv;
> > >
> > > 'sta->drv_priv' is only a pointer, we don't really dereference the
> > > data right here, so I think this is safe. More, compiler can optimize
> > > this instruction that reorder it to the place just right before using.
> > > So, it seems like a false alarm.
> > 
> > The warning is about "sta" not "sta->priv".  It's not a false positive.
> > 
> > I have heard discussions about compilers trying to work around these
> > bugs by re-ordering the code.  Is that an option in GCC?  It's not
> > something we should rely on, but I'm just curious if it exists in
> > released versions.
> > 
> 
> I say GCC does "reorder" the code, because the object codes of following
> two codes are identical with default or -Os ccflags.

Huh...  That's cool.  GCC doesn't re-order it for me, but I'm on GCC 8
so maybe it will work when I get to a more modern version.

regards,
dan carpenter