linux-kernel - Re: [PATCH] mtd: nand: raw: qcom_nandc: handle error pointer from adm prep_slave

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: <4dcb0e76-b965-42da-b637-751d2f8e1c51@www.fastmail.com>
Date:   Fri, 16 Sep 2022 11:01:11 +0200
From:   "Arnd Bergmann" <arnd@...db.de>
To:     "Christian Marangi" <ansuelsmth@...il.com>,
        "Manivannan Sadhasivam" <mani@...nel.org>,
        "Miquel Raynal" <miquel.raynal@...tlin.com>,
        "Richard Weinberger" <richard@....at>,
        "Vignesh Raghavendra" <vigneshr@...com>,
        "Vinod Koul" <vkoul@...nel.org>, "Mark Brown" <broonie@...nel.org>,
        linux-mtd@...ts.infradead.org, linux-arm-msm@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     stable@...r.kernel.org
Subject: Re: [PATCH] mtd: nand: raw: qcom_nandc: handle error pointer from adm
 prep_slave_sg

On Fri, Sep 16, 2022, at 2:10 AM, Christian Marangi wrote:
> ADM dma engine has changed to also provide error pointer instaed of
> plain NULL pointer on invalid configuration of prep_slave_sg function.
> Currently this is not handled and an error pointer is detected as a
> valid dma_desc. This cause kernel panic as the driver doesn't fail
> with an invalid dma engine configuration.
>
> Correctly handle this case by checking if dma_desc is NULL or IS_ERR.

Using IS_ERR_OR_NULL() is almost never a correct solution. I think
in this case the problem is the adm_prep_slave_sg() function
that returns an invalid error code.

While error pointers are often better than NULL pointers for
passing information to the caller, a driver can't just change
the calling conventions on its own. If we want to change
the dmaengine_prep_slave_sg() API, I would suggest coming
up with a new name for a replacement interface that uses
error pointers instead of NULL first, and then changing
all callers to the new interface.

       Arnd