Re: [RFC] mtd: Fix error code loss in mtdchar_read() function.

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.
Date: Mon, 25 Sep 2023 11:28:53 +0200	[thread overview]
Message-ID: <20230925112853.6cb7fff3@xps-13> (raw)
In-Reply-To: <495954216.80155.1695633280285.JavaMail.zimbra@nod.at>

Hi Richard,

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

> ----- Ursprüngliche Mail -----
> >> 'total_retlen' is 0, not the error code.  
> > 
> > Actually after looking at the code, I have no strong opinion
> > regarding whether we should return 0 or an error code in this case.
> > 
> > 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:
> > 
> >                /* 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 -EUCLEAN
> >                 * 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
> >                 */
> >   
> >> 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.  
> 
> This is a special purpose device file and not a regular file.
> Please explain in detail why this violates POSIX and which program breaks.
> 
> 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=217939

Thanks,
Miquèl

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/