From: Dan Carpenter <dan.carpenter@oracle.com>
To: Saurav Girepunje <saurav.girepunje@gmail.com>
Cc: Larry.Finger@lwfinger.net, florian.c.schilhabel@googlemail.com,
gregkh@linuxfoundation.org, fmdefrancesco@gmail.com,
linux-staging@lists.linux.dev, linux-kernel@vger.kernel.org,
saurav.girepunje@hotmail.com
Subject: Re: [PATCH v2] staging: rtl8712: Move similar execution in to a function.
Date: Fri, 10 Sep 2021 12:00:45 +0300 [thread overview]
Message-ID: <20210910090045.GD7203@kadam> (raw)
In-Reply-To: <YTsXXxtQn7QN6nIm@user>
On Fri, Sep 10, 2021 at 01:59:19PM +0530, Saurav Girepunje wrote:
> In rtl8712_cmd.c function read_macreg_hdl,write_macreg_hdl,write_bbreg_hdl
> and write_rfreg_hdl all are having same execution.
I get what you're trying to do, because this code is bad and duplicative
but this is not the right fix.
Let's take read_macreg_hdl() as an example.
Look at how it's called:
215 switch (pcmd->cmdcode) {
216 case GEN_CMD_CODE(_Read_MACREG):
217 read_macreg_hdl(padapter, (u8 *)pcmd);
218 pcmd_r = pcmd;
219 break;
Then look at how it's implemented:
120 static u8 read_macreg_hdl(struct _adapter *padapter, u8 *pbuf)
121 {
122 void (*pcmd_callback)(struct _adapter *dev, struct cmd_obj *pcmd);
123 struct cmd_obj *pcmd = (struct cmd_obj *)pbuf;
124
125 /* invoke cmd->callback function */
126 pcmd_callback = cmd_callback[pcmd->cmdcode].callback;
So pcmd->cmdcode is GEN_CMD_CODE(_Read_MACREG). We look that up in the
cmd_callback[] array and it is:
{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
127 if (!pcmd_callback)
128 r8712_free_cmd_obj(pcmd);
So now we no that "pcmd_callback" is NULL meaning it will free "pcmd".
And if you remember in the caller it does "pcmd_r = pcmd;" but "pcmd"
is freed so that's going to lead to a use after free in r8712_cmd_thread().
It's garbage and the patch doesn't really help.
The right way to fix it is to get rid of the cmd_callback[] array.
129 else
130 pcmd_callback(padapter, pcmd);
131 return H2C_SUCCESS;
132 }
Getting rid of the cmd_callback[] array looks like this. In
read_rfreg_hdl() we know that the callback is r8712_getbbrfreg_cmdrsp_callback()
so we can call that directly.
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c b/drivers/staging/rtl8712/rtl8712_cmd.c
index e9294e1ed06e..51a6abb27d41 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -174,11 +174,7 @@ static u8 read_rfreg_hdl(struct _adapter *padapter, u8 *pbuf)
if (pcmd->rsp && pcmd->rspsz > 0)
memcpy(pcmd->rsp, (u8 *)&val, pcmd->rspsz);
- pcmd_callback = cmd_callback[pcmd->cmdcode].callback;
- if (!pcmd_callback)
- r8712_free_cmd_obj(pcmd);
- else
- pcmd_callback(padapter, pcmd);
+ r8712_getbbrfreg_cmdrsp_callback(padapter, pcmd);
return H2C_SUCCESS;
}
regards,
dan carpenter
next prev parent reply other threads:[~2021-09-10 9:01 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-09-10 8:29 [PATCH v2] staging: rtl8712: Move similar execution in to a function Saurav Girepunje
2021-09-10 9:00 ` Dan Carpenter [this message]
2021-09-10 16:49 ` Saurav Girepunje
2021-09-11 7:30 ` Dan Carpenter
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20210910090045.GD7203@kadam \
--to=dan.carpenter@oracle.com \
--cc=Larry.Finger@lwfinger.net \
--cc=florian.c.schilhabel@googlemail.com \
--cc=fmdefrancesco@gmail.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-staging@lists.linux.dev \
--cc=saurav.girepunje@gmail.com \
--cc=saurav.girepunje@hotmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.