* [PATCH] media: videobuf2-core: Fix error handling when fileio is deallocated @ 2018-11-12 0:49 ` Myungho Jung 2018-11-13 10:27 ` Marek Szyprowski 0 siblings, 1 reply; 4+ messages in thread From: Myungho Jung @ 2018-11-12 0:49 UTC (permalink / raw) To: pawel, m.szyprowski, kyungmin.park, mchehab; +Cc: linux-media, linux-kernel The mutex that is held from vb2_fop_read() can be unlocked while waiting for a buffer if the queue is streaming and blocking. Meanwhile, fileio can be released. So, it should return an error if the fileio address is changed. Signed-off-by: Myungho Jung <mhjungk@gmail.com> Reported-by: syzbot+4180ff9ca6810b06c1e9@syzkaller.appspotmail.com --- drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c index 975ff5669f72..bff94752eb27 100644 --- a/drivers/media/common/videobuf2/videobuf2-core.c +++ b/drivers/media/common/videobuf2/videobuf2-core.c @@ -2564,6 +2564,10 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ dprintk(5, "vb2_dqbuf result: %d\n", ret); if (ret) return ret; + if (fileio != q->fileio) { + dprintk(3, "fileio deallocated\n"); + return -EFAULT; + } fileio->dq_count += 1; fileio->cur_index = index; -- 2.17.1 ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] media: videobuf2-core: Fix error handling when fileio is deallocated 2018-11-12 0:49 ` [PATCH] media: videobuf2-core: Fix error handling when fileio is deallocated Myungho Jung @ 2018-11-13 10:27 ` Marek Szyprowski 2018-11-13 11:56 ` Hans Verkuil 0 siblings, 1 reply; 4+ messages in thread From: Marek Szyprowski @ 2018-11-13 10:27 UTC (permalink / raw) To: Myungho Jung, pawel, kyungmin.park, mchehab; +Cc: linux-media, linux-kernel Hi Myungho, On 2018-11-12 01:49, Myungho Jung wrote: > The mutex that is held from vb2_fop_read() can be unlocked while waiting > for a buffer if the queue is streaming and blocking. Meanwhile, fileio > can be released. So, it should return an error if the fileio address is > changed. > > Signed-off-by: Myungho Jung <mhjungk@gmail.com> > Reported-by: syzbot+4180ff9ca6810b06c1e9@syzkaller.appspotmail.com Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> Thanks for analyzing the code and fixing this issue! > --- > drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > index 975ff5669f72..bff94752eb27 100644 > --- a/drivers/media/common/videobuf2/videobuf2-core.c > +++ b/drivers/media/common/videobuf2/videobuf2-core.c > @@ -2564,6 +2564,10 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ > dprintk(5, "vb2_dqbuf result: %d\n", ret); > if (ret) > return ret; > + if (fileio != q->fileio) { > + dprintk(3, "fileio deallocated\n"); > + return -EFAULT; > + } > fileio->dq_count += 1; > > fileio->cur_index = index; Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: videobuf2-core: Fix error handling when fileio is deallocated 2018-11-13 10:27 ` Marek Szyprowski @ 2018-11-13 11:56 ` Hans Verkuil 2018-11-13 17:38 ` Myungho Jung 0 siblings, 1 reply; 4+ messages in thread From: Hans Verkuil @ 2018-11-13 11:56 UTC (permalink / raw) To: Marek Szyprowski, Myungho Jung, pawel, kyungmin.park, mchehab Cc: linux-media, linux-kernel On 11/13/18 11:27, Marek Szyprowski wrote: > Hi Myungho, > > On 2018-11-12 01:49, Myungho Jung wrote: >> The mutex that is held from vb2_fop_read() can be unlocked while waiting >> for a buffer if the queue is streaming and blocking. Meanwhile, fileio >> can be released. So, it should return an error if the fileio address is >> changed. >> >> Signed-off-by: Myungho Jung <mhjungk@gmail.com> >> Reported-by: syzbot+4180ff9ca6810b06c1e9@syzkaller.appspotmail.com > > Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> Sorry: Nacked-by: Hans Verkuil <hans.verkuil@cisco.com> This addresses the symptom, not the underlying cause. I have a patch that fixes the actual cause that I plan to post soon after I review it a bit more. Regards, Hans > > Thanks for analyzing the code and fixing this issue! > >> --- >> drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c >> index 975ff5669f72..bff94752eb27 100644 >> --- a/drivers/media/common/videobuf2/videobuf2-core.c >> +++ b/drivers/media/common/videobuf2/videobuf2-core.c >> @@ -2564,6 +2564,10 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ >> dprintk(5, "vb2_dqbuf result: %d\n", ret); >> if (ret) >> return ret; >> + if (fileio != q->fileio) { >> + dprintk(3, "fileio deallocated\n"); >> + return -EFAULT; >> + } >> fileio->dq_count += 1; >> >> fileio->cur_index = index; > > Best regards > ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] media: videobuf2-core: Fix error handling when fileio is deallocated 2018-11-13 11:56 ` Hans Verkuil @ 2018-11-13 17:38 ` Myungho Jung 0 siblings, 0 replies; 4+ messages in thread From: Myungho Jung @ 2018-11-13 17:38 UTC (permalink / raw) To: Hans Verkuil, Marek Szyprowski, pawel, kyungmin.park, mchehab Cc: linux-media, linux-kernel On Tue, Nov 13, 2018 at 12:56:18PM +0100, Hans Verkuil wrote: > On 11/13/18 11:27, Marek Szyprowski wrote: > > Hi Myungho, > > > > On 2018-11-12 01:49, Myungho Jung wrote: > >> The mutex that is held from vb2_fop_read() can be unlocked while waiting > >> for a buffer if the queue is streaming and blocking. Meanwhile, fileio > >> can be released. So, it should return an error if the fileio address is > >> changed. > >> > >> Signed-off-by: Myungho Jung <mhjungk@gmail.com> > >> Reported-by: syzbot+4180ff9ca6810b06c1e9@syzkaller.appspotmail.com > > > > Acked-by: Marek Szyprowski <m.szyprowski@samsung.com> > > Sorry: > > Nacked-by: Hans Verkuil <hans.verkuil@cisco.com> > > This addresses the symptom, not the underlying cause. > > I have a patch that fixes the actual cause that I plan to post soon > after I review it a bit more. > > Regards, > > Hans > Hi Hans, Thanks for explaining the root cause. I was also thinking a similar patch with your second one. It looks like the reported syzbot is needed to be added to your first patch. Regards, Myungho > > > > Thanks for analyzing the code and fixing this issue! > > > >> --- > >> drivers/media/common/videobuf2/videobuf2-core.c | 4 ++++ > >> 1 file changed, 4 insertions(+) > >> > >> diff --git a/drivers/media/common/videobuf2/videobuf2-core.c b/drivers/media/common/videobuf2/videobuf2-core.c > >> index 975ff5669f72..bff94752eb27 100644 > >> --- a/drivers/media/common/videobuf2/videobuf2-core.c > >> +++ b/drivers/media/common/videobuf2/videobuf2-core.c > >> @@ -2564,6 +2564,10 @@ static size_t __vb2_perform_fileio(struct vb2_queue *q, char __user *data, size_ > >> dprintk(5, "vb2_dqbuf result: %d\n", ret); > >> if (ret) > >> return ret; > >> + if (fileio != q->fileio) { > >> + dprintk(3, "fileio deallocated\n"); > >> + return -EFAULT; > >> + } > >> fileio->dq_count += 1; > >> > >> fileio->cur_index = index; > > > > Best regards > > > ^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-11-13 17:38 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20181112005053epcas4p1c674759797b4a930cfcce3abc7edd9ad@epcas4p1.samsung.com>
2018-11-12 0:49 ` [PATCH] media: videobuf2-core: Fix error handling when fileio is deallocated Myungho Jung
2018-11-13 10:27 ` Marek Szyprowski
2018-11-13 11:56 ` Hans Verkuil
2018-11-13 17:38 ` Myungho Jung
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.