Bug 241929 - ses(4) fails to sanitize illegal strings in SES element descriptors
Summary: ses(4) fails to sanitize illegal strings in SES element descriptors
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Many People
Assignee: Alan Somers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-11-12 21:34 UTC by Alan Somers
Modified: 2019-12-06 00:06 UTC (History)
5 users (show)

See Also:


Attachments
sanitize SES element descriptors in ses(4) (1.58 KB, patch)
2019-11-13 20:26 UTC, Alan Somers
no flags Details | Diff
sanitize SES element descriptors in ses(4) (1.71 KB, patch)
2019-11-13 20:50 UTC, Alan Somers
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alan Somers freebsd_committer 2019-11-12 21:34:56 UTC
"sesutil --libxo" is awesome.  It's very easy to parse, but ONLY if the enclosure's element descriptors are all valid UTF-8.  Unfortunately, we can't rely on that.  If an element descriptor is not valid UTF-8, then the JSON or XML output is not parseable.  Here's a real-world example:

$ sesutil map --libxo xml,pretty -u /dev/ses0
<sesutil version="1">
  <enclosures>
    <enc>ses0</enc>
    <name>SMC946 C1 0a01</name>
    ...
    <id>500304800924823f</id>
    <elements>
      <id>33</id>
      <type>Enclosure</type>
      <status>OK</status>
      <status_code>0x01 0x00 0x00 0x00</status_code>
      <description>��������������������������������</description>
    </elements>
    ...
  </enclosures>
</sesutil>

Those mystery characters are all 0xFF bytes.  sesutil needs to sanitize those fields before it prints them.
Comment 1 Allan Jude freebsd_committer 2019-11-12 23:13:33 UTC
Thanks for the report, I'll take a look
Comment 2 Alan Somers freebsd_committer 2019-11-12 23:32:24 UTC
No need, allanjude; I'm on it.  I'm thinking that this is actually a kernel bug instead of a sesutil bug.  ses(4) should probably sanitize those fields as soon as it reads them.  Otherwise other programs besides just sesutil could be affected.
Comment 3 Alan Somers freebsd_committer 2019-11-13 20:26:03 UTC
It's definitely a kernel problem.  I have a supermicro expander that puts illegal characters in its SES element descriptors (SES diagnostic page 7).  The kernel faithfully forwards that garbage to consumers of the ENCIOC_GETELMDESC ioctl.  While sesutil(8) could theoretically sanitize those descriptors itself, I think it's better for the kernel to do it.  That's what the attached (not fully tested yet) patch does.
Comment 4 Alan Somers freebsd_committer 2019-11-13 20:26:33 UTC
Created attachment 209138 [details]
sanitize SES element descriptors in ses(4)
Comment 5 Juraj Lutter 2019-11-13 20:42:06 UTC
Just for the record,

elmpriv->descr_len = length

should be adjusted accordingly in csase "<invalid>" is returned.
Comment 6 Alan Somers freebsd_committer 2019-11-13 20:50:57 UTC
Created attachment 209139 [details]
sanitize SES element descriptors in ses(4)

Update elmpriv->descr in the case of an invalid descriptor.
Comment 7 Allan Jude freebsd_committer 2019-11-13 22:23:03 UTC
(In reply to Alan Somers from comment #6)
Thanks for the better fix.

Reviewed By: allanjude
Comment 8 commit-hook freebsd_committer 2019-12-06 00:06:10 UTC
A commit references this bug:

Author: asomers
Date: Fri Dec  6 00:06:06 UTC 2019
New revision: 355430
URL: https://svnweb.freebsd.org/changeset/base/355430

Log:
  ses: sanitize illegal strings in SES element descriptors

  The SES4r3 standard requires that element descriptors may only contain ASCII
  characters in the range 0x20 to 0x7e.  Some SuperMicro expanders violate
  that rule.  This patch adds a sanity check to ses(4).  Descriptors in
  violation will be replaced by "<invalid>".

  This patch fixes "sesutil --libxo xml" on such systems.  Previously it would
  generate non-well-formed XML output.

  PR:		241929
  Reviewed by:	allanjude
  MFC after:	2 weeks
  Sponsored by:	Axcient

Changes:
  head/sys/cam/scsi/scsi_enc_ses.c