Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.37 as permitted sender) client-ip=23.128.96.37;
Date:   Mon, 25 Sep 2023 11:28:53 +0200
From:   Miquel Raynal <miquel.raynal@bootlin.com>
To:     Richard Weinberger <richard@nod.at>
Cc:     ZhaoLong Wang <wangzhaolong1@huawei.com>,
        Vignesh Raghavendra <vigneshr@ti.com>,
        linux-mtd <linux-mtd@lists.infradead.org>,
        linux-kernel <linux-kernel@vger.kernel.org>,
        chengzhihao1 <chengzhihao1@huawei.com>,
        yi zhang <yi.zhang@huawei.com>, yangerkun@huawei.com
Subject: Re: [RFC] mtd: Fix error code loss in mtdchar_read() function.
Message-ID: <20230925112853.6cb7fff3@xps-13>
In-Reply-To: <495954216.80155.1695633280285.JavaMail.zimbra@nod.at>
References: <20230923005856.2538223-1-wangzhaolong1@huawei.com>
        <20230925104938.3f7b4284@xps-13>
        <495954216.80155.1695633280285.JavaMail.zimbra@nod.at>
Organization: Bootlin
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Richard,

richard@nod.at wrote on Mon, 25 Sep 2023 11:14:40 +0200 (CEST):

> ----- Urspr=C3=BCngliche Mail -----
> >> 'total_retlen' is 0, not the error code. =20
> >=20
> > Actually after looking at the code, I have no strong opinion
> > regarding whether we should return 0 or an error code in this case.
> >=20
> > There is this comment right above, and I'm not sure it is still up to
> > date because I believe many drivers just don't provide the data upon
> > ECC error:
> >=20
> >                /* Nand returns -EBADMSG on ECC errors, but it returns
> >                 * the data. For our userspace tools it is important
> >                 * to dump areas with ECC errors!
> >                 * For kernel internal usage it also might return -EUCLE=
AN
> >                 * to signal the caller that a bitflip has occurred and =
has
> >                 * been corrected by the ECC algorithm.
> >                 * Userspace software which accesses NAND this way
> >                 * must be aware of the fact that it deals with NAND
> >                 */
> >  =20
> >> This problem causes the user-space program to encounter EOF when it has
> >> not finished reading the mtd partion, and this also violates the read
> >> system call standard in POSIX. =20
>=20
> This is a special purpose device file and not a regular file.
> Please explain in detail why this violates POSIX and which program breaks.
>=20
> As pointed out by Miquel, the comment makes it clean that this behavior is
> on purpose. If we return now all of a sudden -EBADMSG for the described
> scenario we might even break existing MTD userspace.

The bugzilla link in the commit log [1] mentions:

* dd would just stop in the middle without showing errors
  -> we probably don't care, we expect the userspace to know this is
  NAND when dealing with mtd devices directly, dd is not mtd-aware
  anyway.

* ubiformat would loop forever
  -> that one needs attention I guess :)

[1] https://bugzilla.kernel.org/show_bug.cgi?id=3D217939

Thanks,
Miqu=C3=A8l