linux-kernel - Re: [PATCH v4 2/2] dmaengine: tegra210-adma: check for adma max page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ou25ulmzei773cztbc6cj3na6xq646hpuj4l77nct5mscd5c3y@7e4tqnovx6gw>
Date: Wed, 5 Feb 2025 14:24:44 +0100
From: Thierry Reding <thierry.reding@...il.com>
To: Mohan Kumar D <mkumard@...dia.com>
Cc: Jon Hunter <jonathanh@...dia.com>, vkoul@...nel.org, 
	dmaengine@...r.kernel.org, linux-tegra@...r.kernel.org, linux-kernel@...r.kernel.org, 
	stable@...r.kernel.org
Subject: Re: [PATCH v4 2/2] dmaengine: tegra210-adma: check for adma max page

On Wed, Feb 05, 2025 at 06:28:34PM +0530, Mohan Kumar D wrote:
> 
> On 05-02-2025 17:10, Jon Hunter wrote:
> > 
> > 
> > On 05/02/2025 03:31, Mohan Kumar D wrote:
> > > Have additional check for max channel page during the probe
> > > to cover if any offset overshoot happens due to wrong DT
> > > configuration.
> > > 
> > > Fixes: 68811c928f88 ("dmaengine: tegra210-adma: Support channel page")
> > > Cc: stable@...r.kernel.org
> > > Signed-off-by: Mohan Kumar D <mkumard@...dia.com>
> > > ---
> > >   drivers/dma/tegra210-adma.c | 7 ++++++-
> > >   1 file changed, 6 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/dma/tegra210-adma.c b/drivers/dma/tegra210-adma.c
> > > index a0bd4822ed80..801740ad8e0d 100644
> > > --- a/drivers/dma/tegra210-adma.c
> > > +++ b/drivers/dma/tegra210-adma.c
> > > @@ -83,7 +83,9 @@ struct tegra_adma;
> > >    * @nr_channels: Number of DMA channels available.
> > >    * @ch_fifo_size_mask: Mask for FIFO size field.
> > >    * @sreq_index_offset: Slave channel index offset.
> > > + * @max_page: Maximum ADMA Channel Page.
> > >    * @has_outstanding_reqs: If DMA channel can have outstanding
> > > requests.
> > > + * @set_global_pg_config: Global page programming.
> > >    */
> > >   struct tegra_adma_chip_data {
> > >       unsigned int (*adma_get_burst_config)(unsigned int burst_size);
> > > @@ -99,6 +101,7 @@ struct tegra_adma_chip_data {
> > >       unsigned int nr_channels;
> > >       unsigned int ch_fifo_size_mask;
> > >       unsigned int sreq_index_offset;
> > > +    unsigned int max_page;
> > >       bool has_outstanding_reqs;
> > >       void (*set_global_pg_config)(struct tegra_adma *tdma);
> > >   };
> > > @@ -854,6 +857,7 @@ static const struct tegra_adma_chip_data
> > > tegra210_chip_data = {
> > >       .nr_channels        = 22,
> > >       .ch_fifo_size_mask    = 0xf,
> > >       .sreq_index_offset    = 2,
> > > +    .max_page        = 0,
> > >       .has_outstanding_reqs    = false,
> > >       .set_global_pg_config    = NULL,
> > >   };
> > > @@ -871,6 +875,7 @@ static const struct tegra_adma_chip_data
> > > tegra186_chip_data = {
> > >       .nr_channels        = 32,
> > >       .ch_fifo_size_mask    = 0x1f,
> > >       .sreq_index_offset    = 4,
> > > +    .max_page        = 4,
> > >       .has_outstanding_reqs    = true,
> > >       .set_global_pg_config    = tegra186_adma_global_page_config,
> > >   };
> > > @@ -921,7 +926,7 @@ static int tegra_adma_probe(struct
> > > platform_device *pdev)
> > >               page_offset = res_page->start - res_base->start;
> > >               page_no = div_u64(page_offset, cdata->ch_base_offset);
> > >   -            if (WARN_ON(page_no == 0))
> > > +            if (WARN_ON(page_no == 0 || page_no > cdata->max_page))
> > 
> > So no one should ever specify the 'page' region for Tegra210, correct?
> > If they did then this would always fail. I don't know if it is also
> > worth checking if someone has a 'page' region for a device that has
> > max_page == 0?
> Yes, DT binding specifies "page" should not be used for Tegra210 and above
> conditions takes care. I believe above condition should suffice.

Yes, if you get this wrong, the DT validation should already catch it.
We could always make it very clear by adding an explicit check and a
corresponding error message, but I'm not sure it's really worth it in
this case. If we do, then maybe we should add error messages to these
other cases as well.

Thierry

Download attachment "signature.asc" of type "application/pgp-signature" (834 bytes)