From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752909AbdJLCQ4 (ORCPT ); Wed, 11 Oct 2017 22:16:56 -0400 Received: from out1-smtp.messagingengine.com ([66.111.4.25]:46039 "EHLO out1-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752123AbdJLCQz (ORCPT ); Wed, 11 Oct 2017 22:16:55 -0400 X-ME-Sender: Date: Thu, 12 Oct 2017 13:16:47 +1100 From: "Tobin C. Harding" To: Shreeya Patel Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] Staging: rtlwifi: Remove NULL pointer dereference Message-ID: <20171012021647.GB30753@eros> References: <1507583938-13272-1-git-send-email-shreeya.patel23498@gmail.com> <20171010000607.GA29970@eros> <1507725167.2137.5.camel@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1507725167.2137.5.camel@gmail.com> X-Mailer: Mutt 1.5.24 (2015-08-30) User-Agent: Mutt/1.5.24 (2015-08-30) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Oct 11, 2017 at 06:02:47PM +0530, Shreeya Patel wrote: > On Tue, 2017-10-10 at 11:06 +1100, Tobin C. Harding wrote: > > On Tue, Oct 10, 2017 at 02:48:58AM +0530, Shreeya Patel wrote: > > > > > > Remove NULL pointer dereference as it results in undefined > > > behaviour, and will usually lead to a runtime error. > > The diff does not show any pointer dereference so it is hard to > > understand what you are trying to do > > with this patch. > > > > > > > > Signed-off-by: Shreeya Patel > > > --- > > >  drivers/staging/rtlwifi/base.c | 2 +- > > >  1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/drivers/staging/rtlwifi/base.c > > > b/drivers/staging/rtlwifi/base.c > > > index b88b0e8..5bb8f98 100644 > > > --- a/drivers/staging/rtlwifi/base.c > > > +++ b/drivers/staging/rtlwifi/base.c > > > @@ -781,7 +781,7 @@ static void _rtl_txrate_selectmode(struct > > > ieee80211_hw *hw, > > >   > > >   struct rtl_priv *rtlpriv = rtl_priv(hw); > > >   struct rtl_mac *mac = rtl_mac(rtl_priv(hw)); > > > - struct rtl_sta_info *sta_entry = NULL; > > > + struct rtl_sta_info *sta_entry; > > Now the pointer just has garbage in it instead of the testable value > > of NULL. If you are concerned > > with the dereference perhaps you could add a NULL check, again it's > > hard to say without seeing the > > code. > > Hello,  > > Thanks for making me understand.  > > Here is the code after declaration and initialization of sta_entry.  > Will it be good to add a NULL check in this case?  > > struct rtl_sta_info *sta_entry = NULL; > u8 ratr_index = SET_RATE_ID(RATR_INX_WIRELESS_MC); > > if (sta) { > sta_entry = (struct rtl_sta_info *)sta->drv_priv; > ratr_index = sta_entry->ratr_index; > } Later in this function the macro SET_RATE_ID() is called, it relies on sta_entry being NULL if it was not explicitly set. Here is the macro; #define SET_RATE_ID(rate_id) \ ((rtlpriv->cfg->spec_ver & RTL_SPEC_NEW_RATEID) ? \ rtl_mrate_idx_to_arfr_id(hw, rate_id, \ (sta_entry ? sta_entry->wireless_mode : \ WIRELESS_MODE_G)) : \ rate_id) > If we are making a pointer point to NULL then what if any other > variable is already pointing to NULL for some other purpose. > Instead, removing initialization will be good right? A pointer does not _point_ to NULL as such. A NULL pointer has a value of all zero bytes. Have you read (and completed all the exercises) in KnR https://en.wikipedia.org/wiki/The_C_Programming_Language It is, in my opinion, one of the best tech books ever written. If you have any holes in your C knowledge, this is the place to start. Good luck, Tobin.