linux-kernel - Re: [PATCH v9 char-misc-next 1/2] misc: microchip: pci1xxxx: Add support to read and write into PCI1XXXX OTP via NVMEM sysfs

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] [day] [month] [year] [list]

Date:   Tue, 18 Apr 2023 11:18:19 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Michael Walle <michael@...le.cc>
Cc:     VaibhaavRam.TL@...rochip.com, linux-gpio@...r.kernel.org,
        linux-kernel@...r.kernel.org, UNGLinuxDriver@...rochip.com,
        arnd@...db.de
Subject: Re: [PATCH v9 char-misc-next 1/2] misc: microchip: pci1xxxx: Add
 support to read and write into PCI1XXXX OTP via NVMEM sysfs

On Tue, Apr 18, 2023 at 11:00:18AM +0200, Michael Walle wrote:
> Am 2023-04-17 14:47, schrieb VaibhaavRam.TL@...rochip.com:
> > > > +#include <linux/auxiliary_bus.h>
> > > > +#include <linux/bio.h>
> > > > +#include <linux/device.h>
> > > > +#include <linux/iopoll.h>
> > > > +#include <linux/kthread.h>
> > > 
> > > Is this needed? I don't see any threads. Also bio.h. Please double
> > > check
> > > your includes.
> > Ok, Will remove in next version of patch
> > > > +     if (priv != NULL)
> > > > +             rb = priv->reg_base;
> > > > +     else
> > > > +             return -ENODEV;
> > > 
> > > Unneeded check, priv cannot be NULL, right?
> > Ok, I think this can be removed
> > > > +
> > > > +             data = readl(rb +
> > > > MMAP_OTP_OFFSET(OTP_PASS_FAIL_OFFSET));
> > > > +             if (ret < 0 || data & OTP_FAIL_BIT)
> > > > +                     break;
> > > 
> > > No error handling?
> > We have implemented short read which returns count of successful bytes
> > read and therefore userspace will recognise the situation when the
> > requested count is not obtained.
> 
> I'll leave that up to Greg, but I'd prefer a proper error to userspace.
> I'm not sure if you'd need to differentiate between a short read/write
> and an error.

A short read isn't an error, it's just returning the amount of data
present which might be less than the buffer passed in, which is totally
normal.  If there is an error, return an error.

thanks,

greg k-h