On Tue, Mar 23, 2021 at 10:30:16PM +0800, lyl2019@xxxxxxxxxxxxxxxx wrote: > > > > > -----原始邮件----- > > 发件人: "Mika Westerberg" <mika.westerberg@xxxxxxxxxxxxxxx> > > 发送时间: 2021-03-23 22:06:47 (星期二) > > 收件人: "Lv Yunlong" <lyl2019@xxxxxxxxxxxxxxxx> > > 抄送: andreas.noever@xxxxxxxxx, michael.jamet@xxxxxxxxx, YehezkelShB@xxxxxxxxx, linux-usb@xxxxxxxxxxxxxxx, linux-kernel@xxxxxxxxxxxxxxx > > 主题: Re: [PATCH] thunderbolt: Fix a double put in tb_cfg_read_raw > > > > Hi, > > > > On Mon, Mar 22, 2021 at 08:15:12PM -0700, Lv Yunlong wrote: > > > In tb_cfg_read_raw, req is allocated by tb_cfg_request_alloc() > > > with an initial reference. Before calling tb_cfg_request_sync(), > > > there is no refcount inc operation. tb_cfg_request_sync() > > > calls tb_cfg_request(..,req,..) and if the callee failed, > > > the initial reference of req is dropped and req is freed. > > > > > > Later in tb_cfg_read_raw before the err check, > > > tb_cfg_request_put(req) is called again. It may cause error > > > in race. > > > > Hmm, tb_cfg_request() does tb_cfg_request_get() too and in case of error > > it does tb_cfg_request_put(). So the refcount should be fine. What am I > > missing? > > > > > > > > My patch puts tb_cfg_request_put(req) after the err check > > > finished to avoid unexpected result. > > > > > > Signed-off-by: Lv Yunlong <lyl2019@xxxxxxxxxxxxxxxx> > > > --- > > > drivers/thunderbolt/ctl.c | 4 ++-- > > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > > > diff --git a/drivers/thunderbolt/ctl.c b/drivers/thunderbolt/ctl.c > > > index f1aeaff9f368..bb60269c89ab 100644 > > > --- a/drivers/thunderbolt/ctl.c > > > +++ b/drivers/thunderbolt/ctl.c > > > @@ -890,11 +890,11 @@ struct tb_cfg_result tb_cfg_read_raw(struct tb_ctl *ctl, void *buffer, > > > > > > res = tb_cfg_request_sync(ctl, req, timeout_msec); > > > > > > - tb_cfg_request_put(req); > > > - > > > if (res.err != -ETIMEDOUT) > > > break; > > > > > > + tb_cfg_request_put(req); > > > + > > > /* Wait a bit (arbitrary time) until we send a retry */ > > > usleep_range(10, 100); > > > } > > > -- > > > 2.25.1 > > > > > I'm very sorry, i was ashamed that i had missed the tb_cfg_request_get() in tb_cfg_request(). It happens, no worries :)
