linux-kernel - Re: [PATCH] libnvdimm: Fix __nd_ioctl() to check error in cmd

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

Date:   Wed, 7 Nov 2018 21:26:23 +0000
From:   "Kani, Toshi" <toshi.kani@....com>
To:     "dan.j.williams@...el.com" <dan.j.williams@...el.com>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "dave.jiang@...el.com" <dave.jiang@...el.com>,
        "linux-nvdimm@...ts.01.org" <linux-nvdimm@...ts.01.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "vishal.l.verma@...el.com" <vishal.l.verma@...el.com>,
        "Elliott, Robert (Persistent Memory)" <elliott@....com>
Subject: Re: [PATCH] libnvdimm: Fix __nd_ioctl() to check error in cmd_rc

On Wed, 2018-11-07 at 11:34 -0800, Dan Williams wrote:
> On Wed, Nov 7, 2018 at 10:52 AM Toshi Kani <toshi.kani@....com> wrote:
> > 
> > ndctl zero-labels completes with a large number of zeroed nmems when
> > it fails to do zeroing on a protected NVDIMM.
> > 
> >   # ndctl zero-labels nmem1
> >   zeroed 65504 nmems
> > 
> > When an ACPI call completes with error, xlat_status() called from
> > acpi_nfit_ctl() sets error to *cmd_rc.  __nd_ioctl(), however, does
> > not check this error and returns with success.
> > 
> > Fix __nd_ioctl() to check this error in cmd_rc.
> 
> So this arrangement is by design and the bug is in the ndctl utility.
> 
> A successful return code from the ioctl means that the command was
> successfully submitted to firmware. It's then up to userspace to parse
> if there was a command specific error returned in the response
> payload. Automatically returning cmd_rc removes the ability for
> userspace tooling to do its own command specific error handling. With
> this change userspace could no longer be sure if the failure is in the
> submission or the execution of the command, or determine if the
> command response payload is valid.

I see.  I was wondering which side needs to be fixed, and decided to
follow kernel-internal ACPI calls like nvdimm_clear_poison().  I agree
that a command error code is necessary if user space tool needs to deal
with it.  OK, I will look into fixing ndctl.

Thanks,
-Toshi