* [PATCH] thunderbolt: Fix a double put in tb_cfg_read_raw @ 2021-03-23 3:15 Lv Yunlong 2021-03-23 14:06 ` Mika Westerberg 0 siblings, 1 reply; 4+ messages in thread From: Lv Yunlong @ 2021-03-23 3:15 UTC (permalink / raw) To: andreas.noever, michael.jamet, mika.westerberg, YehezkelShB Cc: linux-usb, linux-kernel, Lv Yunlong 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. My patch puts tb_cfg_request_put(req) after the err check finished to avoid unexpected result. Signed-off-by: Lv Yunlong <lyl2019@mail.ustc.edu.cn> --- 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 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] thunderbolt: Fix a double put in tb_cfg_read_raw 2021-03-23 3:15 [PATCH] thunderbolt: Fix a double put in tb_cfg_read_raw Lv Yunlong @ 2021-03-23 14:06 ` Mika Westerberg 2021-03-23 14:30 ` lyl2019 0 siblings, 1 reply; 4+ messages in thread From: Mika Westerberg @ 2021-03-23 14:06 UTC (permalink / raw) To: Lv Yunlong Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, linux-kernel 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@mail.ustc.edu.cn> > --- > 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 > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH] thunderbolt: Fix a double put in tb_cfg_read_raw 2021-03-23 14:06 ` Mika Westerberg @ 2021-03-23 14:30 ` lyl2019 2021-03-23 14:44 ` Mika Westerberg 0 siblings, 1 reply; 4+ messages in thread From: lyl2019 @ 2021-03-23 14:30 UTC (permalink / raw) To: Mika Westerberg Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, linux-kernel > -----原始邮件----- > 发件人: "Mika Westerberg" <mika.westerberg@linux.intel.com> > 发送时间: 2021-03-23 22:06:47 (星期二) > 收件人: "Lv Yunlong" <lyl2019@mail.ustc.edu.cn> > 抄送: andreas.noever@gmail.com, michael.jamet@intel.com, YehezkelShB@gmail.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org > 主题: 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@mail.ustc.edu.cn> > > --- > > 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(). Thanks. ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: Re: [PATCH] thunderbolt: Fix a double put in tb_cfg_read_raw 2021-03-23 14:30 ` lyl2019 @ 2021-03-23 14:44 ` Mika Westerberg 0 siblings, 0 replies; 4+ messages in thread From: Mika Westerberg @ 2021-03-23 14:44 UTC (permalink / raw) To: lyl2019; +Cc: andreas.noever, michael.jamet, YehezkelShB, linux-usb, linux-kernel On Tue, Mar 23, 2021 at 10:30:16PM +0800, lyl2019@mail.ustc.edu.cn wrote: > > > > > -----原始邮件----- > > 发件人: "Mika Westerberg" <mika.westerberg@linux.intel.com> > > 发送时间: 2021-03-23 22:06:47 (星期二) > > 收件人: "Lv Yunlong" <lyl2019@mail.ustc.edu.cn> > > 抄送: andreas.noever@gmail.com, michael.jamet@intel.com, YehezkelShB@gmail.com, linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org > > 主题: 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@mail.ustc.edu.cn> > > > --- > > > 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 :) ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-03-23 14:45 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2021-03-23 3:15 [PATCH] thunderbolt: Fix a double put in tb_cfg_read_raw Lv Yunlong 2021-03-23 14:06 ` Mika Westerberg 2021-03-23 14:30 ` lyl2019 2021-03-23 14:44 ` Mika Westerberg
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox