Re: [PATCH] rapidio: fix rio_dma_transfer error handling

[Ioan Nicu <ioan.nicu.ext@nokia.com>]

Hi,

On Thu, Apr 12, 2018 at 07:44:01PM -0400, Alexandre Bounine wrote:
> 
> On 2018-04-12 05:28 PM, Andrew Morton wrote:
> > On Thu, 12 Apr 2018 17:06:05 +0200 Ioan Nicu <ioan.nicu.ext@nokia.com> wrote:
> > 
> > > Some of the mport_dma_req structure members were initialized late
> > > inside the do_dma_request() function, just before submitting the
> > > request to the dma engine. But we have some error branches before
> > > that. In case of such an error, the code would return on the error
> > > path and trigger the calling of dma_req_free() with a req structure
> > > which is not completely initialized. This causes a NULL pointer
> > > dereference in dma_req_free().
> > > 
> > > This patch fixes these error branches by making sure that all
> > > necessary mport_dma_req structure members are initialized in
> > > rio_dma_transfer() immediately after the request structure gets
> > > allocated.
> > 
> > This sounds like something which someone has actually triggered in a
> > real-world situation.  So I added a cc:stable.  Please let me know if
> > that was inappropriate.
> > 
> > And please remember to always include all information regarding
> > end-user impact when fixing bugs.
> > 
> This bug fix is applicable to versions starting from v4.6

Actually, this is something I broke with my previous patch where I added a
kref to the mport_dma_req structure. Before this patch, all the error paths
were doing kfree(req) instead of kref_put(&req->refcount, dma_req_free).

Now that dma_req_free() is called, it dereferences req->dmach, which is
initialized late in do_dma_request(), so dma_req_free() could be called
with a NULL req->dmach in some cases.

Sorry if I did not make this clear enough in the description.

Regards,
Ioan