[PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Sasha Levin]

From: Jia-Ju Bai <baijiaju@tsinghua.edu.cn>

[ Upstream commit 6499a0db9b0f1e903d52f8244eacc1d4be00eea2 ]

The value av7110->debi_virt is stored in DMA memory, and it is assigned
to data, and thus data[0] can be modified at any time by malicious
hardware. In this case, "if (data[0] < 2)" can be passed, but then
data[0] can be changed into a large number, which may cause buffer
overflow when the code "av7110->ci_slot[data[0]]" is used.

To fix this possible bug, data[0] is assigned to a local variable, which
replaces the use of data[0].

Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn>
Signed-off-by: Sean Young <sean@mess.org>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/media/pci/ttpci/av7110.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/media/pci/ttpci/av7110.c b/drivers/media/pci/ttpci/av7110.c
index d6816effb8786..d02b5fd940c12 100644
--- a/drivers/media/pci/ttpci/av7110.c
+++ b/drivers/media/pci/ttpci/av7110.c
@@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
 	case DATA_CI_GET:
 	{
 		u8 *data = av7110->debi_virt;
+		u8 data_0 = data[0];
 
-		if ((data[0] < 2) && data[2] == 0xff) {
+		if (data_0 < 2 && data[2] == 0xff) {
 			int flags = 0;
 			if (data[5] > 0)
 				flags |= CA_CI_MODULE_PRESENT;
 			if (data[5] > 5)
 				flags |= CA_CI_MODULE_READY;
-			av7110->ci_slot[data[0]].flags = flags;
+			av7110->ci_slot[data_0].flags = flags;
 		} else
 			ci_get_data(&av7110->ci_rbuffer,
 				    av7110->debi_virt,
-- 
2.25.1

[PATCH AUTOSEL 4.19 27/38] cec-api: prevent leaking memory through hole in structure

[Sasha Levin]

From: Hans Verkuil <hverkuil-cisco@xs4all.nl>

[ Upstream commit 6c42227c3467549ddc65efe99c869021d2f4a570 ]

Fix this smatch warning:

drivers/media/cec/core/cec-api.c:156 cec_adap_g_log_addrs() warn: check that 'log_addrs' doesn't leak information (struct has a hole after
'features')

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Mauro Carvalho Chehab <mchehab+huawei@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
 drivers/media/cec/cec-api.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/media/cec/cec-api.c b/drivers/media/cec/cec-api.c
index 4961573850d54..b2b3f779592fd 100644
--- a/drivers/media/cec/cec-api.c
+++ b/drivers/media/cec/cec-api.c
@@ -147,7 +147,13 @@ static long cec_adap_g_log_addrs(struct cec_adapter *adap,
 	struct cec_log_addrs log_addrs;
 
 	mutex_lock(&adap->lock);
-	log_addrs = adap->log_addrs;
+	/*
+	 * We use memcpy here instead of assignment since there is a
+	 * hole at the end of struct cec_log_addrs that an assignment
+	 * might ignore. So when we do copy_to_user() we could leak
+	 * one byte of memory.
+	 */
+	memcpy(&log_addrs, &adap->log_addrs, sizeof(log_addrs));
 	if (!adap->is_configured)
 		memset(log_addrs.log_addr, CEC_LOG_ADDR_INVALID,
 		       sizeof(log_addrs.log_addr));
-- 
2.25.1

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Pavel Machek]

[-- Attachment #1: Type: text/plain, Size: 1409 bytes --]

Hi!

> The value av7110->debi_virt is stored in DMA memory, and it is assigned
> to data, and thus data[0] can be modified at any time by malicious
> hardware. In this case, "if (data[0] < 2)" can be passed, but then
> data[0] can be changed into a large number, which may cause buffer
> overflow when the code "av7110->ci_slot[data[0]]" is used.
> 
> To fix this possible bug, data[0] is assigned to a local variable, which
> replaces the use of data[0].

I'm pretty sure hardware capable of manipulating memory can work
around any such checks, but...

> +++ b/drivers/media/pci/ttpci/av7110.c
> @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
>  	case DATA_CI_GET:
>  	{
>  		u8 *data = av7110->debi_virt;
> +		u8 data_0 = data[0];
>  
> -		if ((data[0] < 2) && data[2] == 0xff) {
> +		if (data_0 < 2 && data[2] == 0xff) {
>  			int flags = 0;
>  			if (data[5] > 0)
>  				flags |= CA_CI_MODULE_PRESENT;
>  			if (data[5] > 5)
>  				flags |= CA_CI_MODULE_READY;
> -			av7110->ci_slot[data[0]].flags = flags;
> +			av7110->ci_slot[data_0].flags = flags;

This does not even do what it says. Compiler is still free to access
data[0] multiple times. It needs READ_ONCE() to be effective.

Best regards,
								Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Laurent Pinchart]

On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
> Hi!
> 
> > The value av7110->debi_virt is stored in DMA memory, and it is assigned
> > to data, and thus data[0] can be modified at any time by malicious
> > hardware. In this case, "if (data[0] < 2)" can be passed, but then
> > data[0] can be changed into a large number, which may cause buffer
> > overflow when the code "av7110->ci_slot[data[0]]" is used.
> > 
> > To fix this possible bug, data[0] is assigned to a local variable, which
> > replaces the use of data[0].
> 
> I'm pretty sure hardware capable of manipulating memory can work
> around any such checks, but...
> 
> > +++ b/drivers/media/pci/ttpci/av7110.c
> > @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
> >  	case DATA_CI_GET:
> >  	{
> >  		u8 *data = av7110->debi_virt;
> > +		u8 data_0 = data[0];
> >  
> > -		if ((data[0] < 2) && data[2] == 0xff) {
> > +		if (data_0 < 2 && data[2] == 0xff) {
> >  			int flags = 0;
> >  			if (data[5] > 0)
> >  				flags |= CA_CI_MODULE_PRESENT;
> >  			if (data[5] > 5)
> >  				flags |= CA_CI_MODULE_READY;
> > -			av7110->ci_slot[data[0]].flags = flags;
> > +			av7110->ci_slot[data_0].flags = flags;
> 
> This does not even do what it says. Compiler is still free to access
> data[0] multiple times. It needs READ_ONCE() to be effective.

Yes, it seems quite dubious to me. If we *really* want to guard against
rogue hardware here, the whole DMA buffer should be copied. I don't
think it's worth it, a rogue PCI device can do much more harm.

-- 
Regards,

Laurent Pinchart

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Sean Young]

On Sat, Aug 29, 2020 at 08:16:00PM +0300, Laurent Pinchart wrote:
> On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
> > Hi!
> > 
> > > The value av7110->debi_virt is stored in DMA memory, and it is assigned
> > > to data, and thus data[0] can be modified at any time by malicious
> > > hardware. In this case, "if (data[0] < 2)" can be passed, but then
> > > data[0] can be changed into a large number, which may cause buffer
> > > overflow when the code "av7110->ci_slot[data[0]]" is used.
> > > 
> > > To fix this possible bug, data[0] is assigned to a local variable, which
> > > replaces the use of data[0].
> > 
> > I'm pretty sure hardware capable of manipulating memory can work
> > around any such checks, but...
> > 
> > > +++ b/drivers/media/pci/ttpci/av7110.c
> > > @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
> > >  	case DATA_CI_GET:
> > >  	{
> > >  		u8 *data = av7110->debi_virt;
> > > +		u8 data_0 = data[0];
> > >  
> > > -		if ((data[0] < 2) && data[2] == 0xff) {
> > > +		if (data_0 < 2 && data[2] == 0xff) {
> > >  			int flags = 0;
> > >  			if (data[5] > 0)
> > >  				flags |= CA_CI_MODULE_PRESENT;
> > >  			if (data[5] > 5)
> > >  				flags |= CA_CI_MODULE_READY;
> > > -			av7110->ci_slot[data[0]].flags = flags;
> > > +			av7110->ci_slot[data_0].flags = flags;
> > 
> > This does not even do what it says. Compiler is still free to access
> > data[0] multiple times. It needs READ_ONCE() to be effective.
> 
> Yes, it seems quite dubious to me. If we *really* want to guard against
> rogue hardware here, the whole DMA buffer should be copied. I don't
> think it's worth it, a rogue PCI device can do much more harm.

That is a good point. I'm not sure what the kernel could do to protect
against a malicious PCI device (that can do dma) so this patch is totally
pointless.

Thanks

Sean

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Jia-Ju Bai]

On 2020/8/29 20:10, Pavel Machek wrote:
> Hi!
>
>> The value av7110->debi_virt is stored in DMA memory, and it is assigned
>> to data, and thus data[0] can be modified at any time by malicious
>> hardware. In this case, "if (data[0] < 2)" can be passed, but then
>> data[0] can be changed into a large number, which may cause buffer
>> overflow when the code "av7110->ci_slot[data[0]]" is used.
>>
>> To fix this possible bug, data[0] is assigned to a local variable, which
>> replaces the use of data[0].
> I'm pretty sure hardware capable of manipulating memory can work
> around any such checks, but...
>
>> +++ b/drivers/media/pci/ttpci/av7110.c
>> @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
>>   	case DATA_CI_GET:
>>   	{
>>   		u8 *data = av7110->debi_virt;
>> +		u8 data_0 = data[0];
>>   
>> -		if ((data[0] < 2) && data[2] == 0xff) {
>> +		if (data_0 < 2 && data[2] == 0xff) {
>>   			int flags = 0;
>>   			if (data[5] > 0)
>>   				flags |= CA_CI_MODULE_PRESENT;
>>   			if (data[5] > 5)
>>   				flags |= CA_CI_MODULE_READY;
>> -			av7110->ci_slot[data[0]].flags = flags;
>> +			av7110->ci_slot[data_0].flags = flags;
> This does not even do what it says. Compiler is still free to access
> data[0] multiple times. It needs READ_ONCE() to be effective.
>
>

Thanks for this advice, I will submit a v2 patch soon.


Best wishes,
Jia-Ju Bai

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Jia-Ju Bai]

On 2020/8/30 1:16, Laurent Pinchart wrote:
> On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
>> Hi!
>>
>>> The value av7110->debi_virt is stored in DMA memory, and it is assigned
>>> to data, and thus data[0] can be modified at any time by malicious
>>> hardware. In this case, "if (data[0] < 2)" can be passed, but then
>>> data[0] can be changed into a large number, which may cause buffer
>>> overflow when the code "av7110->ci_slot[data[0]]" is used.
>>>
>>> To fix this possible bug, data[0] is assigned to a local variable, which
>>> replaces the use of data[0].
>> I'm pretty sure hardware capable of manipulating memory can work
>> around any such checks, but...
>>
>>> +++ b/drivers/media/pci/ttpci/av7110.c
>>> @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
>>>   	case DATA_CI_GET:
>>>   	{
>>>   		u8 *data = av7110->debi_virt;
>>> +		u8 data_0 = data[0];
>>>   
>>> -		if ((data[0] < 2) && data[2] == 0xff) {
>>> +		if (data_0 < 2 && data[2] == 0xff) {
>>>   			int flags = 0;
>>>   			if (data[5] > 0)
>>>   				flags |= CA_CI_MODULE_PRESENT;
>>>   			if (data[5] > 5)
>>>   				flags |= CA_CI_MODULE_READY;
>>> -			av7110->ci_slot[data[0]].flags = flags;
>>> +			av7110->ci_slot[data_0].flags = flags;
>> This does not even do what it says. Compiler is still free to access
>> data[0] multiple times. It needs READ_ONCE() to be effective.
> Yes, it seems quite dubious to me. If we *really* want to guard against
> rogue hardware here, the whole DMA buffer should be copied. I don't
> think it's worth it, a rogue PCI device can do much more harm.
>

 From the original driver code, data[0] is considered to be bad and thus 
it should be checked, because the content of the DMA buffer may be 
problematic.
Based on this consideration, data[0] can be also modified to bypass the 
check, and thus its value should be copied to a local variable for the 
check and use.

I agree with Pavel that the compiler optimization may drop the copying 
operation, and thus READ_ONCE() should be used here.
I will submit a v2 patch soon.


Best wishes,
Jia-Ju Bai

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Laurent Pinchart]

Hi Jia-Ju,

On Sun, Aug 30, 2020 at 03:33:11PM +0800, Jia-Ju Bai wrote:
> On 2020/8/30 1:16, Laurent Pinchart wrote:
> > On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
> >> Hi!
> >>
> >>> The value av7110->debi_virt is stored in DMA memory, and it is assigned
> >>> to data, and thus data[0] can be modified at any time by malicious
> >>> hardware. In this case, "if (data[0] < 2)" can be passed, but then
> >>> data[0] can be changed into a large number, which may cause buffer
> >>> overflow when the code "av7110->ci_slot[data[0]]" is used.
> >>>
> >>> To fix this possible bug, data[0] is assigned to a local variable, which
> >>> replaces the use of data[0].
> >> I'm pretty sure hardware capable of manipulating memory can work
> >> around any such checks, but...
> >>
> >>> +++ b/drivers/media/pci/ttpci/av7110.c
> >>> @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
> >>>   	case DATA_CI_GET:
> >>>   	{
> >>>   		u8 *data = av7110->debi_virt;
> >>> +		u8 data_0 = data[0];
> >>>   
> >>> -		if ((data[0] < 2) && data[2] == 0xff) {
> >>> +		if (data_0 < 2 && data[2] == 0xff) {
> >>>   			int flags = 0;
> >>>   			if (data[5] > 0)
> >>>   				flags |= CA_CI_MODULE_PRESENT;
> >>>   			if (data[5] > 5)
> >>>   				flags |= CA_CI_MODULE_READY;
> >>> -			av7110->ci_slot[data[0]].flags = flags;
> >>> +			av7110->ci_slot[data_0].flags = flags;
> >>
> >> This does not even do what it says. Compiler is still free to access
> >> data[0] multiple times. It needs READ_ONCE() to be effective.
> >
> > Yes, it seems quite dubious to me. If we *really* want to guard against
> > rogue hardware here, the whole DMA buffer should be copied. I don't
> > think it's worth it, a rogue PCI device can do much more harm.
> 
> From the original driver code, data[0] is considered to be bad and thus 
> it should be checked, because the content of the DMA buffer may be 
> problematic.
>
> Based on this consideration, data[0] can be also modified to bypass the 
> check, and thus its value should be copied to a local variable for the 
> check and use.

What makes you think the hardware would do that ?

> I agree with Pavel that the compiler optimization may drop the copying 
> operation, and thus READ_ONCE() should be used here.
> I will submit a v2 patch soon.

-- 
Regards,

Laurent Pinchart

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Jia-Ju Bai]

On 2020/8/31 6:25, Laurent Pinchart wrote:
> Hi Jia-Ju,
>
> On Sun, Aug 30, 2020 at 03:33:11PM +0800, Jia-Ju Bai wrote:
>> On 2020/8/30 1:16, Laurent Pinchart wrote:
>>> On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
>>>> Hi!
>>>>
>>>>> The value av7110->debi_virt is stored in DMA memory, and it is assigned
>>>>> to data, and thus data[0] can be modified at any time by malicious
>>>>> hardware. In this case, "if (data[0] < 2)" can be passed, but then
>>>>> data[0] can be changed into a large number, which may cause buffer
>>>>> overflow when the code "av7110->ci_slot[data[0]]" is used.
>>>>>
>>>>> To fix this possible bug, data[0] is assigned to a local variable, which
>>>>> replaces the use of data[0].
>>>> I'm pretty sure hardware capable of manipulating memory can work
>>>> around any such checks, but...
>>>>
>>>>> +++ b/drivers/media/pci/ttpci/av7110.c
>>>>> @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
>>>>>    	case DATA_CI_GET:
>>>>>    	{
>>>>>    		u8 *data = av7110->debi_virt;
>>>>> +		u8 data_0 = data[0];
>>>>>    
>>>>> -		if ((data[0] < 2) && data[2] == 0xff) {
>>>>> +		if (data_0 < 2 && data[2] == 0xff) {
>>>>>    			int flags = 0;
>>>>>    			if (data[5] > 0)
>>>>>    				flags |= CA_CI_MODULE_PRESENT;
>>>>>    			if (data[5] > 5)
>>>>>    				flags |= CA_CI_MODULE_READY;
>>>>> -			av7110->ci_slot[data[0]].flags = flags;
>>>>> +			av7110->ci_slot[data_0].flags = flags;
>>>> This does not even do what it says. Compiler is still free to access
>>>> data[0] multiple times. It needs READ_ONCE() to be effective.
>>> Yes, it seems quite dubious to me. If we *really* want to guard against
>>> rogue hardware here, the whole DMA buffer should be copied. I don't
>>> think it's worth it, a rogue PCI device can do much more harm.
>>  From the original driver code, data[0] is considered to be bad and thus
>> it should be checked, because the content of the DMA buffer may be
>> problematic.
>>
>> Based on this consideration, data[0] can be also modified to bypass the
>> check, and thus its value should be copied to a local variable for the
>> check and use.
> What makes you think the hardware would do that ?
>

Several recent papers show that the bad values from malicious or 
problematic hardware can cause security problems:
[NDSS'19] PeriScope: An Effective Probing and Fuzzing Framework for the 
Hardware-OS Boundary
[NDSS'19] Thunderclap: Exploring Vulnerabilities in Operating System 
IOMMU Protection via DMA from Untrustworthy Peripherals
[USENIX Security'20] USBFuzz: A Framework for Fuzzing USB Drivers by 
Device Emulation

In this case, the values from DMA can be bad, and the driver should 
carefully check these values to avoid security problems.
IOMMU is an effective method to prevent the hardware from accessing 
arbitrary memory address via DMA, but it does not check whether the 
values from DMA are safe.

I find that some drivers (including the av7110 driver) check (or try to 
check) the values from DMA, and thus I think these drivers have 
considered such security problems.
However, some of these checks are not rigorous, so that they can be 
bypassed in some cases. The problem that I reported is such an example.


Best wishes,
Jia-Ju Bai

Re: [PATCH AUTOSEL 4.19 08/38] media: pci: ttpci: av7110: fix possible buffer overflow caused by bad DMA value in debiirq()

[Laurent Pinchart]

Hi Jia-Ju,

On Mon, Aug 31, 2020 at 09:45:14PM +0800, Jia-Ju Bai wrote:
> On 2020/8/31 6:25, Laurent Pinchart wrote:
> > On Sun, Aug 30, 2020 at 03:33:11PM +0800, Jia-Ju Bai wrote:
> >> On 2020/8/30 1:16, Laurent Pinchart wrote:
> >>> On Sat, Aug 29, 2020 at 02:10:20PM +0200, Pavel Machek wrote:
> >>>> Hi!
> >>>>
> >>>>> The value av7110->debi_virt is stored in DMA memory, and it is assigned
> >>>>> to data, and thus data[0] can be modified at any time by malicious
> >>>>> hardware. In this case, "if (data[0] < 2)" can be passed, but then
> >>>>> data[0] can be changed into a large number, which may cause buffer
> >>>>> overflow when the code "av7110->ci_slot[data[0]]" is used.
> >>>>>
> >>>>> To fix this possible bug, data[0] is assigned to a local variable, which
> >>>>> replaces the use of data[0].
> >>>>
> >>>> I'm pretty sure hardware capable of manipulating memory can work
> >>>> around any such checks, but...
> >>>>
> >>>>> +++ b/drivers/media/pci/ttpci/av7110.c
> >>>>> @@ -424,14 +424,15 @@ static void debiirq(unsigned long cookie)
> >>>>>    	case DATA_CI_GET:
> >>>>>    	{
> >>>>>    		u8 *data = av7110->debi_virt;
> >>>>> +		u8 data_0 = data[0];
> >>>>>    
> >>>>> -		if ((data[0] < 2) && data[2] == 0xff) {
> >>>>> +		if (data_0 < 2 && data[2] == 0xff) {
> >>>>>    			int flags = 0;
> >>>>>    			if (data[5] > 0)
> >>>>>    				flags |= CA_CI_MODULE_PRESENT;
> >>>>>    			if (data[5] > 5)
> >>>>>    				flags |= CA_CI_MODULE_READY;
> >>>>> -			av7110->ci_slot[data[0]].flags = flags;
> >>>>> +			av7110->ci_slot[data_0].flags = flags;
> >>>>
> >>>> This does not even do what it says. Compiler is still free to access
> >>>> data[0] multiple times. It needs READ_ONCE() to be effective.
> >>>
> >>> Yes, it seems quite dubious to me. If we *really* want to guard against
> >>> rogue hardware here, the whole DMA buffer should be copied. I don't
> >>> think it's worth it, a rogue PCI device can do much more harm.
> >>
> >>  From the original driver code, data[0] is considered to be bad and thus
> >> it should be checked, because the content of the DMA buffer may be
> >> problematic.
> >>
> >> Based on this consideration, data[0] can be also modified to bypass the
> >> check, and thus its value should be copied to a local variable for the
> >> check and use.
> >
> > What makes you think the hardware would do that ?
> 
> Several recent papers show that the bad values from malicious or 
> problematic hardware can cause security problems:
> [NDSS'19] PeriScope: An Effective Probing and Fuzzing Framework for the 
> Hardware-OS Boundary
> [NDSS'19] Thunderclap: Exploring Vulnerabilities in Operating System 
> IOMMU Protection via DMA from Untrustworthy Peripherals
> [USENIX Security'20] USBFuzz: A Framework for Fuzzing USB Drivers by 
> Device Emulation
> 
> In this case, the values from DMA can be bad, and the driver should 
> carefully check these values to avoid security problems.
> IOMMU is an effective method to prevent the hardware from accessing 
> arbitrary memory address via DMA, but it does not check whether the 
> values from DMA are safe.
> 
> I find that some drivers (including the av7110 driver) check (or try to 
> check) the values from DMA, and thus I think these drivers have 
> considered such security problems.
> However, some of these checks are not rigorous, so that they can be 
> bypassed in some cases. The problem that I reported is such an example.

The AV7110 is an old chip, I'm not even sure if it can be used with a
modern system that supports IOMMUs for PCI devices. Without that, it's
game over anyway. Before trying to address the issue of a malicious
AV7110 playing with DMA and CPU races, I would ensure that it's worth
it.

-- 
Regards,

Laurent Pinchart