Re: [PATCH/RFC 1/5] mmc: renesas-sdhi, tmio: make dma more modular

[Simon Horman <horms@verge.net.au>]

On Tue, May 02, 2017 at 02:57:23PM +0200, Simon Horman wrote:
> On Tue, May 02, 2017 at 02:01:28PM +0200, Arnd Bergmann wrote:
> > On Tue, May 2, 2017 at 1:15 PM, Simon Horman <horms+renesas@verge.net.au> wrote:
> > 
> > > @@ -202,6 +214,15 @@ void tmio_mmc_enable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> > >  void tmio_mmc_disable_mmc_irqs(struct tmio_mmc_host *host, u32 i);
> > >  irqreturn_t tmio_mmc_irq(int irq, void *devid);
> > >
> > > +#if IS_ENABLED(CONFIG_MMC_SDHI)
> > > +const struct tmio_mmc_dma_ops *tmio_mmc_get_dma_ops(void);
> > > +#else
> > > +static inline const struct tmio_mmc_dma_ops *tmio_mmc_get_dma_ops(void)
> > > +{
> > > +       return NULL;
> > > +}
> > > +#endif
> > > +
> > >  static inline char *tmio_mmc_kmap_atomic(struct scatterlist *sg,
> > >                                          unsigned long *flags)
> > >  {
> > 
> > Here you return a NULL pointer for the operations structure
> > 
> > > +
> > > +static inline void tmio_mmc_enable_dma(struct tmio_mmc_host *host, bool enable)
> > > +{
> > > +       if (host->dma_ops->enable)
> > > +               host->dma_ops->enable(host, enable);
> > > +}
> > >
> > 
> > And here you check the ->enable callback but not the dma_ops pointer.
> > In the other callbacks you check the dma_ops pointer but not the callback.
> > Is that intentional? Maybe always check the dma_ops pointer first for
> > consistency, as a NULL operations structure (resulting from a future bug)
> > might lead running user space with kernel privileges.
> 
> Thanks for noticing.
> 
> I resolved that problem but it seems to have crept back in again.
> I'll check to see how that happened.

I take back my comment above - I fixed a different problem and it looks
like I sent out the latest local patches (phew!).

Regardless, I'll look into the problem you mention with a view to fixing it
in v2.