From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752809AbdJMSzL (ORCPT ); Fri, 13 Oct 2017 14:55:11 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:43492 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751541AbdJMSzJ (ORCPT ); Fri, 13 Oct 2017 14:55:09 -0400 X-Google-Smtp-Source: AOwi7QC5/Kkco98wJdKuDd9LxRMA36Ygmr01qsBapVgmbRF1tPNpAWjSbkUiGMZYsLCR6b/PRU6gGQ== Message-ID: <1507920904.2087.2.camel@gmail.com> Subject: Re: [PATCH] Staging: rtlwifi: Remove NULL pointer dereference From: Shreeya Patel To: "Tobin C. Harding" Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org Date: Sat, 14 Oct 2017 00:25:04 +0530 In-Reply-To: <20171012021647.GB30753@eros> References: <1507583938-13272-1-git-send-email-shreeya.patel23498@gmail.com> <20171010000607.GA29970@eros> <1507725167.2137.5.camel@gmail.com> <20171012021647.GB30753@eros> Content-Type: text/plain; charset="UTF-8" X-Mailer: Evolution 3.18.5.2-0ubuntu3.2 Mime-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2017-10-12 at 13:16 +1100, Tobin C. Harding wrote: > 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. Thank you so much. I will make sure that I don't make the same mistake again. > > Good luck, > Tobin.