[PATCH] fabrics: fix return value of hostnqn_read_file()

[PATCH] fabrics: fix return value of hostnqn_read_file()

[Chaitanya Kulkarni]

From: Chaitanya Kulkarni <kch@nvidia.com>

Current code returns the value of strdup and callers of
hostnqn_read_file() check for the NULL value in order to determine the
success or failure.

The value return by the strdup will never be NULL unless strdup()
results in an error that is making function return success all the time
resulting in blktests failures.

Check the string value to NULL before we return the pointer.

Signed-off-by: Chaitanya Kulkarni <kch@nvidia.com>
---
Test Log:-

root@dev nvme-cli ((HEAD detached at v1.15)) # cdblktests 
root@dev blktests (master) # ./check tests/nvme/004
nvme/004 (test nvme and nvmet UUID NS descriptors)           [failed]
    runtime  1.451s  ...  0.072s
    --- tests/nvme/004.out	2021-08-29 01:09:20.287901783 -0700
    +++ /mnt/data/blktests/results/nodev/nvme/004.out.bad	2021-09-05 17:47:34.344388492 -0700
    @@ -1,5 +1,6 @@
     Running nvme/004
    -91fdba0d-f87b-4c25-b80f-db7be1418b9e
    -uuid.91fdba0d-f87b-4c25-b80f-db7be1418b9e
    -NQN:blktests-subsystem-1 disconnected 1 controller(s)
    +Failed to write to /dev/nvme-fabrics: Invalid argument
    +cat: /sys/block/n1/uuid: No such file or directory
    +cat: /sys/block/n1/wwid: No such file or directory
    ...
    (Run 'diff -u tests/nvme/004.out /mnt/data/blktests/results/nodev/nvme/004.out.bad' to see the entire diff)
root@dev blktests (master) # cat /mnt/data/blktests/results/nodev/nvme/004.out.bad
Running nvme/004
Failed to write to /dev/nvme-fabrics: Invalid argument
cat: /sys/block/n1/uuid: No such file or directory
cat: /sys/block/n1/wwid: No such file or directory
NQN:blktests-subsystem-1 disconnected 0 controller(s)
Test complete
root@dev blktests (master) # dmesg  -c 
[187767.848420] run blktests nvme/004 at 2021-09-05 17:47:34
[187767.870446] loop0: detected capacity change from 0 to 2097152
[187767.885230] nvmet: adding nsid 1 to subsystem blktests-subsystem-1
[187767.888098] nvme_fabrics: unknown parameter or missing value 'hostnqn=' in ctrl creation request
root@dev blktests (master) # cd ../
root@dev data # cd nvme-cli/
root@dev nvme-cli ((HEAD detached at v1.15)) # git am 0001-fabrics-fix-return-value-of-hostnqn_read_file.patch
Applying: fabrics: fix return value of hostnqn_read_file()
root@dev nvme-cli ((HEAD detached from v1.15)) # makej install 
NVME_VERSION = 1.15.1.ga91e
    CC fabrics.o
make -C Documentation install-no-build
install -d /usr/share/bash-completion/completions
install -d /usr/share/zsh/site-functions
install -d /etc/nvme
install -d /usr/lib/systemd/system
install -d /etc/udev/rules.d
install -d /usr/lib/dracut/dracut.conf.d
install -m 644 -T ./completions/_nvme /usr/share/zsh/site-functions/_nvme
install -m 644 -T ./completions/bash-nvme-completion.sh /usr/share/bash-completion/completions/nvme
touch /etc/nvme/hostnqn
install -m 644 ./nvmf-autoconnect/systemd/* /usr/lib/systemd/system
install -m 644 ./nvmf-autoconnect/udev-rules/* /etc/udev/rules.d
install -m 644 70-nvmf-autoconnect.conf /usr/lib/dracut/dracut.conf.d
touch /etc/nvme/hostid
if [ ! -f /etc/nvme/discovery.conf ]; then \
	install -m 644 -T ./etc/discovery.conf.in /etc/nvme/discovery.conf; \
fi
if [ ! -s /etc/nvme/hostnqn ]; then \
	echo `/usr/sbin/nvme gen-hostnqn` > /etc/nvme/hostnqn; \
fi
if [ ! -s /etc/nvme/hostid ]; then \
	uuidgen > /etc/nvme/hostid; \
fi
make[1]: Entering directory '/mnt/data/nvme-cli/Documentation'
install -d -m 755 /usr/local/share/man/man1
install -m 644 nvme-admin-passthru.1 nvme-ana-log.1 nvme-attach-ns.1 nvme-capacity-mgmt.1 nvme-changed-ns-list-log.1 nvme-compare.1 nvme-connect-all.1 nvme-connect.1 nvme-copy.1 nvme-create-ns.1 nvme-delete-ns.1 nvme-dera-stat.1 nvme-detach-ns.1 nvme-device-self-test.1 nvme-dir-receive.1 nvme-dir-send.1 nvme-disconnect-all.1 nvme-disconnect.1 nvme-discover.1 nvme-dsm.1 nvme-effects-log.1 nvme-endurance-event-agg-log.1 nvme-endurance-log.1 nvme-error-log.1 nvme-flush.1 nvme-format.1 nvme-fw-commit.1 nvme-fw-download.1 nvme-fw-log.1 nvme-gen-hostnqn.1 nvme-get-feature.1 nvme-get-lba-status.1 nvme-get-log.1 nvme-get-ns-id.1 nvme-get-property.1 nvme-help.1 nvme-huawei-id-ctrl.1 nvme-huawei-list.1 nvme-id-ctrl.1 nvme-id-domain.1 nvme-id-iocs.1 nvme-id-ns.1 nvme-id-nvmset.1 nvme-intel-id-ctrl.1 nvme-intel-internal-log.1 nvme-intel-lat-stats.1 nvme-intel-market-name.1 nvme-intel-smart-log-add.1 nvme-intel-temp-stats.1 nvme-io-passthru.1 nvme-lba-status-log.1 nvme-list-ctrl.1 nvme-list-ns.1 nvme-list-subsys.1 nvme-list.1 nvme-lnvm-create.1 nvme-lnvm-diag-bbtbl.1 nvme-lnvm-diag-set-bbtbl.1 nvme-lnvm-factory.1 nvme-lnvm-id-ns.1 nvme-lnvm-info.1 nvme-lnvm-init.1 nvme-lnvm-list.1 nvme-lnvm-remove.1 nvme-micron-clear-pcie-errors.1 nvme-micron-internal-log.1 nvme-micron-nand-stats.1 nvme-micron-pcie-stats.1 nvme-micron-selective-download.1 nvme-micron-smart-add-log.1 nvme-micron-temperature-stats.1 nvme-netapp-ontapdevices.1 nvme-netapp-smdevices.1 nvme-ns-descs.1 nvme-ns-rescan.1 nvme-nvm-id-ctrl.1 nvme-persistent-event-log.1 nvme-predictable-lat-log.1 nvme-pred-lat-event-agg-log.1 nvme-primary-ctrl-caps.1 nvme-read.1 nvme-reset.1 nvme-resv-acquire.1 nvme-resv-notif-log.1 nvme-resv-register.1 nvme-resv-release.1 nvme-resv-report.1 nvme-rpmb.1 nvme-sanitize-log.1 nvme-sanitize.1 nvme-security-recv.1 nvme-security-send.1 nvme-self-test-log.1 nvme-set-feature.1 nvme-set-property.1 nvme-show-hostnqn.1 nvme-show-regs.1 nvme-smart-log.1 nvme-subsystem-reset.1 nvme-telemetry-log.1 nvme-toshiba-clear-pcie-correctable-errors.1 nvme-toshiba-vs-internal-log.1 nvme-toshiba-vs-smart-add-log.1 nvme-transcend-badblock.1 nvme-transcend-healthvalue.1 nvme-verify.1 nvme-virtium-save-smart-to-vtview-log.1 nvme-virtium-show-identify.1 nvme-wdc-capabilities.1 nvme-wdc-cap-diag.1 nvme-wdc-clear-assert-dump.1 nvme-wdc-clear-fw-activate-history.1 nvme-wdc-clear-pcie-correctable-errors.1 nvme-wdc-cloud-SSD-plugin-version.1 nvme-wdc-drive-essentials.1 nvme-wdc-drive-log.1 nvme-wdc-drive-resize.1 nvme-wdc-enc-get-log.1 nvme-wdc-get-crash-dump.1 nvme-wdc-get-drive-status.1 nvme-wdc-get-pfail-dump.1 nvme-wdc-id-ctrl.1 nvme-wdc-log-page-directory.1 nvme-wdc-namespace-resize.1 nvme-wdc-purge-monitor.1 nvme-wdc-purge.1 nvme-wdc-vs-drive-info.1 nvme-wdc-vs-error-reason-identifier.1 nvme-wdc-vs-fw-activate-history.1 nvme-wdc-vs-internal-log.1 nvme-wdc-vs-nand-stats.1 nvme-wdc-vs-smart-add-log.1 nvme-wdc-vs-telemetry-controller-option.1 nvme-wdc-vs-temperature-stats.1 nvme-write.1 nvme-write-uncor.1 nvme-write-zeroes.1 nvme-zns-changed-zone-list.1 nvme-zns-close-zone.1 nvme-zns-finish-zone.1 nvme-zns-id-ctrl.1 nvme-zns-id-ns.1 nvme-zns-offline-zone.1 nvme-zns-open-zone.1 nvme-zns-report-zones.1 nvme-zns-reset-zone.1 nvme-zns-set-zone-desc.1 nvme-zns-zone-append.1 nvme-zns-zone-mgmt-recv.1 nvme-zns-zone-mgmt-send.1 nvme.1 /usr/local/share/man/man1
make[1]: Leaving directory '/mnt/data/nvme-cli/Documentation'
    CC nvme
rm -f /usr/local/sbin/nvme
install -d /usr/sbin
install -m 755 nvme /usr/sbin
root@dev nvme-cli ((HEAD detached from v1.15)) # cd ../
root@dev data # cdblktests 
root@dev blktests (master) # !./check
./check tests/nvme/004
nvme/004 (test nvme and nvmet UUID NS descriptors)           [passed]
    runtime  0.072s  ...  1.441s
---
 fabrics.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fabrics.c b/fabrics.c
index 39bee97..4e1ff61 100644
--- a/fabrics.c
+++ b/fabrics.c
@@ -855,7 +855,7 @@ static char *hostnqn_read_file(void)
 
 out:
 	fclose(f);
-	return ret;
+	return strcmp(ret, "") == 0 ? NULL : ret;
 }
 
 static char *hostnqn_generate_systemd(void)
-- 
2.29.0


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] fabrics: fix return value of hostnqn_read_file()

[Keith Busch]

On Sun, Sep 05, 2021 at 05:57:19PM -0700, Chaitanya Kulkarni wrote:
> @@ -855,7 +855,7 @@ static char *hostnqn_read_file(void)
>  
>  out:
>  	fclose(f);
> -	return ret;
> +	return strcmp(ret, "") == 0 ? NULL : ret;
>  }

But if the 'goto out' label is used, you are calling strcmp() with a
NULL pointer.

And if "ret" is an emptry string, I think you still need to free() it
here since it is not being returned anymore.

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

Re: [PATCH] fabrics: fix return value of hostnqn_read_file()

[Chaitanya Kulkarni]

On 9/7/21 2:29 PM, Keith Busch wrote:
> External email: Use caution opening links or attachments
> 
> 
> On Sun, Sep 05, 2021 at 05:57:19PM -0700, Chaitanya Kulkarni wrote:
>> @@ -855,7 +855,7 @@ static char *hostnqn_read_file(void)
>>
>>   out:
>>        fclose(f);
>> -     return ret;
>> +     return strcmp(ret, "") == 0 ? NULL : ret;
>>   }
> 
> But if the 'goto out' label is used, you are calling strcmp() with a
> NULL pointer.
> 
> And if "ret" is an emptry string, I think you still need to free() it
> here since it is not being returned anymore.
> 

Let me resend a new patch as this patch is wrong.


_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme