Messages in this thread | | | Date | Tue, 5 May 2020 19:02:33 +0200 | From | Greg KH <> | Subject | Re: [BUG] net: chelsio: Possible buffer overflow caused by DMA failures/attacks |
| |
On Tue, May 05, 2020 at 11:50:28PM +0800, Jia-Ju Bai wrote: > In alloc_rx_resources(): > sge->respQ.entries = > pci_alloc_consistent(pdev, size, &sge->respQ.dma_addr); > > Thus, "sge->respQ.entries" is a DMA value, and it is assigned to > "e" in process_pure_responses(): > struct sge *sge = adapter->sge; > struct respQ *q = &sge->respQ; > struct respQ_e *e = &q->entries[q->cidx]; > > When DMA failures or attacks occur, the data stored in "e" can be > changed at any time. In this case, the value of "e->FreelistQid" > can be a large number to cause buffer overflow when the > following code is executed: > const struct freelQ *fl = &sge->freelQ[e->FreelistQid]; > > Similarly, "sge->respQ.entries" is also assigned to "e" in > process_responses(): > struct sge *sge = adapter->sge; > struct respQ *q = &sge->respQ; > struct respQ_e *e = &q->entries[q->cidx]; > > When DMA failures or attacks occur, the data stored in "e" can be > changed at any time. In this case, the value of "e->FreelistQid" > can be a large number to cause buffer overflow when the > following code is executed: > struct freelQ *fl = &sge->freelQ[e->FreelistQid]; > > Considering that DMA can fail or be attacked, I think that it is dangerous > to > use a DMA value (or any value tainted by it) as an array index or a > control-flow > condition. However, I have found many such dangerous cases in Linux device > drivers > through my static-analysis tool and code review. > I am not sure whether my opinion is correct, so I want to listen to your > points of view.
Can you create a patch to show what you think needs to be fixed? That's the best way to get feedback, reports like this are usually very infrequently replied to.
thanks,
greg k-h
| |