Page MenuHomeFreeBSD

Fix non-printable characters in NVMe model and serial numbers.
ClosedPublic

Authored by ken on Jan 20 2022, 8:18 PM.

Details

Summary
The NVMe 1.4 spec simply says that Model and Serial numbers are
ASCII strings.  Unlike SCSI, it doesn't prohibit non-printable
characters or say that the strings should be padded with spaces.

Since 2014, we have had cam_strvis_sbuf(), which gives additional
options for handling non-ASCII characters.  That behavior hasn't
been available for non-sbuf consumers, so users of cam_strvis()
were left with having octal ASCII codes inserted.

So, to avoid having garbage or octal chracters in the strings, use
cam_strvis_sbuf() to create a new function, cam_strvis_flag(), and
re-implement cam_strvis() using cam_strvis_flag().

Now, for the NVMe drives, we can use cam_strvis_flag with the
CAM_STRVIS_FLAG_NONASCII_SPC flag.  This transforms non-printable
characters into spaces.

sys/cam/cam.c:
        Add a new function, cam_strvis_flag(), that creates an sbuf
        on the stack with the user's destination buffer, and calls
        cam_strvis_sbuf() with the given flag argument.

        Re-implement cam_strvis() to call cam_strvis_flag with the
        CAM_STRVIS_FLAG_NONASCII_ESC argument.  This should be the
        equivalent of the old cam_strvis() function, except for the
        overhead of creating the sbuf and calling sbuf_putc/printf.

sys/cam/cam.h:
        Declaration for cam_strvis_flag.

sys/cam/nvme/nvme_all.c:
        In nvme_print_ident, use the NONASCII_SPC flag with
        cam_strvis_flag().

sys/cam/nvme/nvme_da.c:
        In ndaregister(), use cam_strvis_flag() with the
        NONASCII_SPC flag for the disk description and serial
        number we report to GEOM.

sys/cam/nvme/nvme_xpt.c:
        In nvme_probe_done(), use cam_strvis_flag with the
        NONASCII_SPC flag when storing the drive serial number
        in the CAM EDT.

MFC after: 1 week
Sponsored by: Spectra Logic

Test Plan

Get a NVMe drive with bogus characters in the Model or Serial number.
Try it out (diskinfo -v, dmesg, camcontrol devlist, etc.) with and
without these changes.

Diff Detail

Repository
R10 FreeBSD src repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

ken requested review of this revision.Jan 20 2022, 8:18 PM
sys/cam/nvme/nvme_da.c
933–938

These three statements are repeated 3 times. Please make it a function since it would also save at least one declarations of sb.

Create a cam_strvis_flag() function and use it in the NVMe code.

This allows specifying alternate handling modes for cam_strvis,
but doesn't require the caller to instantiate an sbuf.

Also, re-implement cam_strvis() using cam_strvis_flag().

This revision is now accepted and ready to land.Jan 25 2022, 6:40 PM

Thanks for the updates. This looks good now.