Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 2017/4/4 1:40, Jaegeuk Kim wrote: > On 04/01, Chao Yu wrote: >> Ping, >> >> Any problem here? >> >> Thanks, >> >> On 2017/3/28 9:17, Chao Yu wrote: >>> On 2017/3/28 7:56, Jaegeuk Kim wrote: On 03/27, Chao Yu wrote: > In f2fs_submit_discard_endio, we will wake up waiter before setting > discard command states, so waiter may use incorrect states. Change > the order between complete() and states setting to fix this issue. > > Signed-off-by: Chao Yu> --- > fs/f2fs/segment.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 57a81f9c8c14..9f9542c9fe47 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) > { > struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; > > - complete(>wait); > dc->error = bio->bi_error; > dc->state = D_DONE; > + complete(>wait); If we set D_DONE first, the object can be released by __remove_discard_cmd()? > > What I mean was about use-after-free. I updated the patch, could you help to review it? Thanks, > > Thanks, > >>> >>> Yes, I think so. >>> >>> Thanks, >>> Thanks, > bio_put(bio); > } > > -- > 2.8.2.295.g3f1c1d0 . > > . >
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 2017/4/4 1:40, Jaegeuk Kim wrote: > On 04/01, Chao Yu wrote: >> Ping, >> >> Any problem here? >> >> Thanks, >> >> On 2017/3/28 9:17, Chao Yu wrote: >>> On 2017/3/28 7:56, Jaegeuk Kim wrote: On 03/27, Chao Yu wrote: > In f2fs_submit_discard_endio, we will wake up waiter before setting > discard command states, so waiter may use incorrect states. Change > the order between complete() and states setting to fix this issue. > > Signed-off-by: Chao Yu > --- > fs/f2fs/segment.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 57a81f9c8c14..9f9542c9fe47 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) > { > struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; > > - complete(>wait); > dc->error = bio->bi_error; > dc->state = D_DONE; > + complete(>wait); If we set D_DONE first, the object can be released by __remove_discard_cmd()? > > What I mean was about use-after-free. I updated the patch, could you help to review it? Thanks, > > Thanks, > >>> >>> Yes, I think so. >>> >>> Thanks, >>> Thanks, > bio_put(bio); > } > > -- > 2.8.2.295.g3f1c1d0 . > > . >
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 04/01, Chao Yu wrote: > Ping, > > Any problem here? > > Thanks, > > On 2017/3/28 9:17, Chao Yu wrote: > > On 2017/3/28 7:56, Jaegeuk Kim wrote: > >> On 03/27, Chao Yu wrote: > >>> In f2fs_submit_discard_endio, we will wake up waiter before setting > >>> discard command states, so waiter may use incorrect states. Change > >>> the order between complete() and states setting to fix this issue. > >>> > >>> Signed-off-by: Chao Yu> >>> --- > >>> fs/f2fs/segment.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > >>> index 57a81f9c8c14..9f9542c9fe47 100644 > >>> --- a/fs/f2fs/segment.c > >>> +++ b/fs/f2fs/segment.c > >>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) > >>> { > >>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; > >>> > >>> - complete(>wait); > >>> dc->error = bio->bi_error; > >>> dc->state = D_DONE; > >>> + complete(>wait); > >> > >> If we set D_DONE first, the object can be released by > >> __remove_discard_cmd()? What I mean was about use-after-free. Thanks, > > > > Yes, I think so. > > > > Thanks, > > > >> > >> Thanks, > >> > >>> bio_put(bio); > >>> } > >>> > >>> -- > >>> 2.8.2.295.g3f1c1d0 > >> > >> . > >>
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 04/01, Chao Yu wrote: > Ping, > > Any problem here? > > Thanks, > > On 2017/3/28 9:17, Chao Yu wrote: > > On 2017/3/28 7:56, Jaegeuk Kim wrote: > >> On 03/27, Chao Yu wrote: > >>> In f2fs_submit_discard_endio, we will wake up waiter before setting > >>> discard command states, so waiter may use incorrect states. Change > >>> the order between complete() and states setting to fix this issue. > >>> > >>> Signed-off-by: Chao Yu > >>> --- > >>> fs/f2fs/segment.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > >>> index 57a81f9c8c14..9f9542c9fe47 100644 > >>> --- a/fs/f2fs/segment.c > >>> +++ b/fs/f2fs/segment.c > >>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) > >>> { > >>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; > >>> > >>> - complete(>wait); > >>> dc->error = bio->bi_error; > >>> dc->state = D_DONE; > >>> + complete(>wait); > >> > >> If we set D_DONE first, the object can be released by > >> __remove_discard_cmd()? What I mean was about use-after-free. Thanks, > > > > Yes, I think so. > > > > Thanks, > > > >> > >> Thanks, > >> > >>> bio_put(bio); > >>> } > >>> > >>> -- > >>> 2.8.2.295.g3f1c1d0 > >> > >> . > >>
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
Ping, Any problem here? Thanks, On 2017/3/28 9:17, Chao Yu wrote: > On 2017/3/28 7:56, Jaegeuk Kim wrote: >> On 03/27, Chao Yu wrote: >>> In f2fs_submit_discard_endio, we will wake up waiter before setting >>> discard command states, so waiter may use incorrect states. Change >>> the order between complete() and states setting to fix this issue. >>> >>> Signed-off-by: Chao Yu>>> --- >>> fs/f2fs/segment.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >>> index 57a81f9c8c14..9f9542c9fe47 100644 >>> --- a/fs/f2fs/segment.c >>> +++ b/fs/f2fs/segment.c >>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) >>> { >>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; >>> >>> - complete(>wait); >>> dc->error = bio->bi_error; >>> dc->state = D_DONE; >>> + complete(>wait); >> >> If we set D_DONE first, the object can be released by __remove_discard_cmd()? > > Yes, I think so. > > Thanks, > >> >> Thanks, >> >>> bio_put(bio); >>> } >>> >>> -- >>> 2.8.2.295.g3f1c1d0 >> >> . >>
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
Ping, Any problem here? Thanks, On 2017/3/28 9:17, Chao Yu wrote: > On 2017/3/28 7:56, Jaegeuk Kim wrote: >> On 03/27, Chao Yu wrote: >>> In f2fs_submit_discard_endio, we will wake up waiter before setting >>> discard command states, so waiter may use incorrect states. Change >>> the order between complete() and states setting to fix this issue. >>> >>> Signed-off-by: Chao Yu >>> --- >>> fs/f2fs/segment.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >>> index 57a81f9c8c14..9f9542c9fe47 100644 >>> --- a/fs/f2fs/segment.c >>> +++ b/fs/f2fs/segment.c >>> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) >>> { >>> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; >>> >>> - complete(>wait); >>> dc->error = bio->bi_error; >>> dc->state = D_DONE; >>> + complete(>wait); >> >> If we set D_DONE first, the object can be released by __remove_discard_cmd()? > > Yes, I think so. > > Thanks, > >> >> Thanks, >> >>> bio_put(bio); >>> } >>> >>> -- >>> 2.8.2.295.g3f1c1d0 >> >> . >>
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 2017/3/28 7:56, Jaegeuk Kim wrote: > On 03/27, Chao Yu wrote: >> In f2fs_submit_discard_endio, we will wake up waiter before setting >> discard command states, so waiter may use incorrect states. Change >> the order between complete() and states setting to fix this issue. >> >> Signed-off-by: Chao Yu>> --- >> fs/f2fs/segment.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >> index 57a81f9c8c14..9f9542c9fe47 100644 >> --- a/fs/f2fs/segment.c >> +++ b/fs/f2fs/segment.c >> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) >> { >> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; >> >> -complete(>wait); >> dc->error = bio->bi_error; >> dc->state = D_DONE; >> +complete(>wait); > > If we set D_DONE first, the object can be released by __remove_discard_cmd()? Yes, I think so. Thanks, > > Thanks, > >> bio_put(bio); >> } >> >> -- >> 2.8.2.295.g3f1c1d0 > > . >
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 2017/3/28 7:56, Jaegeuk Kim wrote: > On 03/27, Chao Yu wrote: >> In f2fs_submit_discard_endio, we will wake up waiter before setting >> discard command states, so waiter may use incorrect states. Change >> the order between complete() and states setting to fix this issue. >> >> Signed-off-by: Chao Yu >> --- >> fs/f2fs/segment.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >> index 57a81f9c8c14..9f9542c9fe47 100644 >> --- a/fs/f2fs/segment.c >> +++ b/fs/f2fs/segment.c >> @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) >> { >> struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; >> >> -complete(>wait); >> dc->error = bio->bi_error; >> dc->state = D_DONE; >> +complete(>wait); > > If we set D_DONE first, the object can be released by __remove_discard_cmd()? Yes, I think so. Thanks, > > Thanks, > >> bio_put(bio); >> } >> >> -- >> 2.8.2.295.g3f1c1d0 > > . >
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 03/27, Chao Yu wrote: > In f2fs_submit_discard_endio, we will wake up waiter before setting > discard command states, so waiter may use incorrect states. Change > the order between complete() and states setting to fix this issue. > > Signed-off-by: Chao Yu> --- > fs/f2fs/segment.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 57a81f9c8c14..9f9542c9fe47 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) > { > struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; > > - complete(>wait); > dc->error = bio->bi_error; > dc->state = D_DONE; > + complete(>wait); If we set D_DONE first, the object can be released by __remove_discard_cmd()? Thanks, > bio_put(bio); > } > > -- > 2.8.2.295.g3f1c1d0
Re: [PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
On 03/27, Chao Yu wrote: > In f2fs_submit_discard_endio, we will wake up waiter before setting > discard command states, so waiter may use incorrect states. Change > the order between complete() and states setting to fix this issue. > > Signed-off-by: Chao Yu > --- > fs/f2fs/segment.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 57a81f9c8c14..9f9542c9fe47 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) > { > struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; > > - complete(>wait); > dc->error = bio->bi_error; > dc->state = D_DONE; > + complete(>wait); If we set D_DONE first, the object can be released by __remove_discard_cmd()? Thanks, > bio_put(bio); > } > > -- > 2.8.2.295.g3f1c1d0
[PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
In f2fs_submit_discard_endio, we will wake up waiter before setting discard command states, so waiter may use incorrect states. Change the order between complete() and states setting to fix this issue. Signed-off-by: Chao Yu--- fs/f2fs/segment.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 57a81f9c8c14..9f9542c9fe47 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) { struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; - complete(>wait); dc->error = bio->bi_error; dc->state = D_DONE; + complete(>wait); bio_put(bio); } -- 2.8.2.295.g3f1c1d0
[PATCH 3/3] f2fs: prevent waiter encountering incorrect discard states
In f2fs_submit_discard_endio, we will wake up waiter before setting discard command states, so waiter may use incorrect states. Change the order between complete() and states setting to fix this issue. Signed-off-by: Chao Yu --- fs/f2fs/segment.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 57a81f9c8c14..9f9542c9fe47 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -717,9 +717,9 @@ static void f2fs_submit_discard_endio(struct bio *bio) { struct discard_cmd *dc = (struct discard_cmd *)bio->bi_private; - complete(>wait); dc->error = bio->bi_error; dc->state = D_DONE; + complete(>wait); bio_put(bio); } -- 2.8.2.295.g3f1c1d0