[Bug 243318] GEOM /dev/diskid '%20' in names

[bugzilla-noreply at freebsd.org]

https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=243318

Conrad Meyer <cem at freebsd.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
           Severity|Affects Some People         |Affects Many People
                 CC|                            |cem at freebsd.org
            Version|12.1-RELEASE                |CURRENT

--- Comment #1 from Conrad Meyer <cem at freebsd.org> ---
The device node comes from GEOM label.  g_label_disk_ident taste gets the
string from g_io_getattr GEOM::ident and does not modify it.  It is then
returned to g_label.c::g_label_taste, 

386                 g_labels[i]->ld_taste(cp, label, sizeof(label));
387                 g_label_mangle_name(label, sizeof(label));
388                 g_topology_lock();
389                 if (label[0] == '\0')
390                         continue;
391                 g_label_create(NULL, mp, pp, label, g_labels[i]->ld_dir,
392                     pp->mediasize);

g_label_mangle_name is what converts spaces into valid /dev names (no spaces):

178 g_label_mangle_name(char *label, size_t size)
179 {
180         struct sbuf *sb;
181         const u_char *c;
182
183         sb = sbuf_new(NULL, NULL, size, SBUF_FIXEDLEN);
184         for (c = label; *c != '\0'; c++) {
185                 if (!isprint(*c) || isspace(*c) || *c =='"' || *c == '%')
186                         sbuf_printf(sb, "%%%02X", *c);
187                 else
188                         sbuf_putc(sb, *c);

Where does GEOM::ident come from?  Only two places: md(4) (not this case) and
cam_xpt (this case):

xpt_getattr:

  1233         struct ccb_dev_advinfo cdai;
  1239         memset(&cdai, 0, sizeof(cdai));
  1240         xpt_setup_ccb(&cdai.ccb_h, path, CAM_PRIORITY_NORMAL);
  1241         cdai.ccb_h.func_code = XPT_DEV_ADVINFO;
  1242         cdai.flags = CDAI_FLAG_NONE;
  1243         cdai.bufsiz = len;
  1244         cdai.buf = buf;
  1245
  1246         if (!strcmp(attr, "GEOM::ident"))
  1247                 cdai.buftype = CDAI_TYPE_SERIAL_NUM;

Ok.  There are 4 CAM transports that handle CDAI_TYPE_SERIAL_NUM: ata, mmc,
nvme, and scsi.  This is scsi.

  2512 scsi_dev_advinfo(union ccb *start_ccb)
  2513 {
  2514         struct cam_ed *device;
...
  2534         case CDAI_TYPE_SERIAL_NUM:
  2543                 memcpy(cdai->buf, device->serial_num, amt);

So... what's setting it in device->serial_num?  cam/scsi/scsi_xpt.c::
probestart() PROBE_SERIAL_NUM -> probedone PROBE_SERIAL_NUM:

We try to trim off leading spaces:

  1574         case PROBE_SERIAL_NUM:
  1575         {
...
  1603                                 start = strspn(serial_buf->serial_num, "
");
  1604                                 slen = serial_buf->length - start;
...
  1613                                 memcpy(path->device->serial_num,
  1614                                        &serial_buf->serial_num[start],
slen);
  1615                                 path->device->serial_num_len = slen;
  1616                                 path->device->serial_num[slen] = '\0';

But not trailing spaces, AFAICT.

So, it seems to me like we should probably trim trailing spaces at some layer. 
It could be done in CAM, since the serial number probably does end in literal
spaces (fixed size field).  It could be done in g_label, but that would impact
all g_label devices.  (I'm not sure that's a bad thing.)  If you label your GPT
partition "foo     ", is it really helpful to have /dev/gpt/foo%20%20%20%20?  I
suspect not.

-- 
You are receiving this mail because:
You are the assignee for the bug.