linux-kernel - Re: [PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit arguments and variables used for addressing.

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 for Android: free password hash cracker in your pocket

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

Date:   Mon, 6 Aug 2018 15:01:37 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     Boris Brezillon <boris.brezillon@...tlin.com>
Cc:     Jheng-Jhong Wu <goodwater.wu@...il.com>,
        Palle Christensen <palle.christensen@...rosemi.com>,
        devel@...verdev.osuosl.org,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org,
        Masahiro Yamada <yamada.masahiro@...ionext.com>,
        Arun Nagendran <arunrasppi@...il.com>,
        Miquel Raynal <miquel.raynal@...tlin.com>
Subject: Re: [PATCH] staging:mt29f_spinand: MT29F2G failing as only 16-bit
 arguments and variables used for addressing.

On Mon, Aug 06, 2018 at 01:46:48PM +0200, Boris Brezillon wrote:
> Hi Dan,
> 
> On Wed, 1 Aug 2018 15:05:51 +0300
> Dan Carpenter <dan.carpenter@...cle.com> wrote:
> 
> > On Wed, Aug 01, 2018 at 11:24:19AM +0800, Jheng-Jhong Wu wrote:
> > > For NAND flash chips with more than 1Gbit (e.g. MT29F2G) more than 16 bits
> > > are necessary to address the correct page. The driver sets the address for
> > > more than 16 bits, but it uses 16-bit arguments and variables (these are
> > > page_id, block_id, row) to do address operations. Obviously, these
> > > arguments and variables cannot deal with more than 16-bit address.
> > > 
> > > Signed-off-by: Jheng-Jhong Wu <goodwater.wu@...il.com>  
> > 
> > This seems reasonable...  It would be needed to make commit 6efb21d6d0e7
> > ("staging:mt29f_spinand: MT29F2G failing as only 16 bits used for
> > addressing.") work.  It also fixes a static checker warning.
> > 
> > My only concern is that the mtd/nand code seems to use -1 as a magical
> > page_id.  For example:
> 
> Yes, -1 means "don't issue the row/page address cycles", though I
> don't think page can be -1 for NAND_CMD_READ{1,0} commands.
> 

It sure looks like it can be in nand_exit_status_op()...

> Anyway, if you want this patch merged to fix a static checker warning,
> I'm fine with that. In any case, I still plan to send a patch removing
> this driver for v4.20, so, anyone using this driver should start
> testing the new SPI NAND driver (drivers/mtd/nand/spi) and tweak/fix
> the new implementation if needed.

I don't think we should make the code more complicated than necessary
just to make static checkers happy.  When you say "this driver", you
mean the staging driver?  In that case, there is no need to revert
commit 6efb21d6d0e7 ("staging:mt29f_spinand: MT29F2G failing as only 16
bits used for addressing.").

regards,
dan carpenter