On Wed, Jun 12, 2019 at 04:26:16PM -0700, Verma, Vishal L wrote:
>
> On Wed, 2019-06-12 at 16:20 -0700, Dan Williams wrote:
> > On Wed, Jun 12, 2019 at 4:05 PM Alison Schofield
> > <[email protected]> wrote:
> > > Fix a typo in security.sh that causes a script failure
> > > when an nvdimm-master.blob already exists and needs to
> > > be backed up.
> > >
> > > + setup_keys
> > > + '[' '!' -d /etc/ndctl/keys ']'
> > > + '[' -f /etc/ndctl/keys/nvdimm-master.blob ']'
> > > + mv /etc/ndctl/keys/nvdimm-master.blob
> > > /etc/ndctl/keys/nvdimm-master.blob.bak
> > > + 0=1
> > > ./security.sh: line 39: 0=1: command not found
> > >
> > > Fixes: ba35642d3815 ("ndctl: add a load-keys test in the security unit
> > > test")
> > > Signed-off-by: Alison Schofield <[email protected]>
> > > ---
> > > test/security.sh | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/test/security.sh b/test/security.sh
> > > index 8a36265..c86d2c6 100755
> > > --- a/test/security.sh
> > > +++ b/test/security.sh
> > > @@ -36,11 +36,11 @@ setup_keys()
> > >
> > > if [ -f "$masterpath" ]; then
> > > mv "$masterpath" "$masterpath.bak"
> > > - $backup_key=1
> > > + backup_key=1
> > > fi
> > > if [ -f "$keypath/tpm.handle" ]; then
> > > mv "$keypath/tpm.handle" "$keypath/tmp.handle.bak"
> > > - $backup_handle=1
> > > + backup_handle=1
> > > fi
> >
> > Looks obviously correct to me.
> >
> > Reviewed-by: Dan Williams <[email protected]>
> >
> > ...but that said, why is this test even bothering with the host's
> > configuration? I think it should be using a test local directory that
> > does not disturb the rest of the system, especially because the test
> > is using nfit_test resources.
At first glance, it appears that the keys need to be in the
{ndctl_keysdir}, aka, the official system location, for some
of the ndctl commands to run. So, it's not as simple as just
creating the key blob in a temp directory.
And, I don't even think that's the nfit_test resource you are
referring to anyway. I'll keep a look out for how it can run
cleaner, and make it off the ENABLE_DESTRUCTIVE list in the future.
> >
> > There's no guarantee that the script successfully reaches the
> > post_cleanup() phase to restore the host configuration and could leave
> > it broken. Unless / until we can fix up this test to not touch /etc I
> > think it should be moved to the ENABLE_DESTRUCTIVE set of tests.
>
> Hm, yes good point. I agree with moving it to destructive for now.
Vishal, Do you need a patch that moves it to the naughty list?
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm
