| Message ID | 20200830082042.17462-1-baijiaju@tsinghua.edu.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | media: pci: ttpci: av7110: avoid compiler optimization of reading data[0] in debiirq() | expand |
On Sun, Aug 30, 2020 at 04:20:42PM +0800, Jia-Ju Bai wrote: > In debiirq(), data_0 stores the value of data[0], but it can be dropped > by compiler optimization. Thus, data[0] is read through READ_ONCE(). > > Fixes: 6499a0db9b0f ("media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()") > Reported-by: Pavel Machek <pavel@ucw.cz> Pavel reported that your patch was garbage, if you are trying to defend against a malicious pci device. READ_ONCE() will not help here. Sean > Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn> > --- > drivers/media/pci/ttpci/av7110.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/media/pci/ttpci/av7110.c b/drivers/media/pci/ttpci/av7110.c > index bf36b1e22b63..f7d098d5b198 100644 > --- a/drivers/media/pci/ttpci/av7110.c > +++ b/drivers/media/pci/ttpci/av7110.c > @@ -406,7 +406,7 @@ static void debiirq(unsigned long cookie) > case DATA_CI_GET: > { > u8 *data = av7110->debi_virt; > - u8 data_0 = data[0]; > + u8 data_0 = READ_ONCE(data[0]); > > if (data_0 < 2 && data[2] == 0xff) { > int flags = 0; > -- > 2.17.1
On Sun 2020-08-30 09:30:36, Sean Young wrote: > On Sun, Aug 30, 2020 at 04:20:42PM +0800, Jia-Ju Bai wrote: > > In debiirq(), data_0 stores the value of data[0], but it can be dropped > > by compiler optimization. Thus, data[0] is read through READ_ONCE(). > > > > Fixes: 6499a0db9b0f ("media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()") > > Reported-by: Pavel Machek <pavel@ucw.cz> > > Pavel reported that your patch was garbage, if you are trying to defend > against a malicious pci device. READ_ONCE() will not help here. I would not use exactly those words, but agreed; we should have some explanation that it is feasible to protect against malicious av7110 device, first. Best regards, Pavel
diff --git a/drivers/media/pci/ttpci/av7110.c b/drivers/media/pci/ttpci/av7110.c index bf36b1e22b63..f7d098d5b198 100644 --- a/drivers/media/pci/ttpci/av7110.c +++ b/drivers/media/pci/ttpci/av7110.c @@ -406,7 +406,7 @@ static void debiirq(unsigned long cookie) case DATA_CI_GET: { u8 *data = av7110->debi_virt; - u8 data_0 = data[0]; + u8 data_0 = READ_ONCE(data[0]); if (data_0 < 2 && data[2] == 0xff) { int flags = 0;
In debiirq(), data_0 stores the value of data[0], but it can be dropped by compiler optimization. Thus, data[0] is read through READ_ONCE(). Fixes: 6499a0db9b0f ("media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()") Reported-by: Pavel Machek <pavel@ucw.cz> Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn> --- drivers/media/pci/ttpci/av7110.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)