ctld: Fix coverity raised issues

[thj]

Summary

Only use strtoul when str is not NULL, this addresses a coverity
warning.

Use strlcpy rather than strncpy

Only copy the lun backend string when it is valid

Test Plan

I tested that this runs and mounts with the handbook example:
https://people.freebsd.org/~blackend/en_US.ISO8859-1/books/handbook/network-iscsi.html

[markj]

usr.sbin/ctld/kernel.c

369

log_errx() never returns, so str can't be NULL here. Same below.

[pstef]

usr.sbin/ctld/kernel.c

368

Here and in the other two places you check if str is not null, but I think it would read clearer if you added the cur_port->pp = 0; to the existing conditional expression and, after an else, the cur_port->pp = strtoul(str, NULL, 0);.

[thj]

Hits coverity id: 1474355 in our coverity

[thj]

usr.sbin/ctld/kernel.c

369

great I will drop these patches

[thj]

• Revert "ctld: Only use strtoul when str is not NULL"

[markj]

usr.sbin/ctld/kernel.c

700

I don't think serial_num or device_id is expected to be NULL terminated, i.e., they are not C strings, so the change looks wrong since it might truncate the last byte by inserting a nul terminator.

790

When can l_backend be NULL? Why don't kernel_lun_add() or kernel_lun_remove() need similar treatment?

[jhb]

usr.sbin/ctld/kernel.c

368

If log_err* aren't marked as __dead2, then adding that might help Coverity to avoid adding false positives. Hmm, both log_err() and log_errx() are marked as __dead2, so not sure why Coverity didn't grok that?

700

Confirmed by digging around in the ctl code that the destination are fixed-length buffers that do not necessarily hold C strings. We need to avoid overflowing the source as the source is a C string (hence using strncpy and not memcpy), but the target does not need a nul.

790

It is never NULL. In other places in ctld we always give it a value, e.g. in conf_verify_lun in ctld.c:

	if (lun->l_backend == NULL)
		lun_set_backend(lun, "block");

[imp]

usr.sbin/ctld/kernel.c

700

strncpy is the right thing here because it's a fixed length string. That's what it was written for. Same for model, etc.

707

also a fixed length string, so strncpy is right.