Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 11 Dec 2019 09:42:56 -0800
From:      Warner Losh <imp@bsdimp.com>
To:        Alan Somers <asomers@freebsd.org>
Cc:        Scott Long <scottl@samsco.org>, Steven Hartland <steven.hartland@multiplay.co.uk>,  src-committers <src-committers@freebsd.org>, svn-src-all <svn-src-all@freebsd.org>,  svn-src-head <svn-src-head@freebsd.org>
Subject:   Re: svn commit: r355430 - head/sys/cam/scsi
Message-ID:  <CANCZdfoVZOVUGQ1VPtEbaYkwPPADK-2K5pn_qpE5amEOQtY4VA@mail.gmail.com>
In-Reply-To: <CAOtMX2jWMGfDBTObFQqtDYxQZTykhu%2B6XUL5_=ScfdVUPXe%2B-Q@mail.gmail.com>
References:  <201912060006.xB6066qR058963@repo.freebsd.org> <CAHEMsqaO7SrMQyXVzUdNSvDBTxFwD95s5i2dK=h5an-xvrdcgA@mail.gmail.com> <CAOtMX2iySx2sn2q9qm2FBudpAEBZX6UkaCR3P%2B%2BDGBjntUG=Zg@mail.gmail.com> <820BE55B-AE32-44E7-8AC7-245EF6F86F8B@samsco.org> <CAOtMX2jWMGfDBTObFQqtDYxQZTykhu%2B6XUL5_=ScfdVUPXe%2B-Q@mail.gmail.com>

next in thread | previous in thread | raw e-mail | index | archive | help
At some point, we used to replace bad chars with spaces, but that may have
been two ata stacks ago..

Warner

On Wed, Dec 11, 2019, 9:35 AM Alan Somers <asomers@freebsd.org> wrote:

> No, and there's no possibility of connecting a Windows host to this
> particular device.  We have some Oracle Solaris servers hooked up to thes=
e
> expanders, but it looks like Solaris completely ignores the offending
> element type.
> -Alan
>
> On Wed, Dec 11, 2019 at 10:19 AM Scott Long <scottl@samsco.org> wrote:
>
>> U+FFFD doesn=E2=80=99t make sense for an ASCII string, but 0x3F might.  =
Any idea
>> what Windows shows for this device?
>>
>> Scott
>>
>> > On Dec 11, 2019, at 8:42 AM, Alan Somers <asomers@freebsd.org> wrote:
>> >
>> > In this case the offending descriptor is solid 0xFF, so replacing
>> individual characters wouldn't accomplish anything.  I can imagine a
>> different buggy expander that has just one or two bad characters.  In th=
at
>> case, it would make sense to replace them.  But replace them with what?
>> The UTF replacement character 0xFFFD isn't an option, because the result=
 is
>> supposed to be ASCII.  There's no other obvious choice, which is why I
>> chose to replace the whole thing.
>> > -Alan
>> >
>> > On Fri, Dec 6, 2019 at 2:40 AM Steven Hartland <
>> steven.hartland@multiplay.co.uk> wrote:
>> > If the illegal chars where removed or replaced would the result be
>> useful, if so might that be a better approach?
>> >
>> > On Fri, 6 Dec 2019 at 00:06, Alan Somers <asomers@freebsd.org> wrote:
>> > Author: asomers
>> > Date: Fri Dec  6 00:06:05 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 i=
n
>> >   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
>> >
>> > Modified:
>> >   head/sys/cam/scsi/scsi_enc_ses.c
>> >
>> > Modified: head/sys/cam/scsi/scsi_enc_ses.c
>> >
>> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D
>> > --- head/sys/cam/scsi/scsi_enc_ses.c    Thu Dec  5 19:39:51 2019
>> (r355429)
>> > +++ head/sys/cam/scsi/scsi_enc_ses.c    Fri Dec  6 00:06:05 2019
>> (r355430)
>> > @@ -110,7 +110,7 @@ typedef struct ses_addl_status {
>> >  typedef struct ses_element {
>> >         uint8_t eip;                    /* eip bit is set */
>> >         uint16_t descr_len;             /* length of the descriptor */
>> > -       char *descr;                    /* descriptor for this object =
*/
>> > +       const char *descr;              /* descriptor for this object =
*/
>> >         struct ses_addl_status addl;    /* additional status info */
>> >  } ses_element_t;
>> >
>> > @@ -1977,6 +1977,35 @@ ses_publish_cache(enc_softc_t *enc, struct
>> enc_fsm_sta
>> >         return (0);
>> >  }
>> >
>> > +/*
>> > + * \brief Sanitize an element descriptor
>> > + *
>> > + * The SES4r3 standard, sections 3.1.2 and 6.1.10, specifies that
>> element
>> > + * descriptors may only contain ASCII characters in the range 0x20 to
>> 0x7e.
>> > + * But some vendors violate that rule.  Ensure that we only expose
>> compliant
>> > + * descriptors to userland.
>> > + *
>> > + * \param desc         SES element descriptor as reported by the
>> hardware
>> > + * \param len          Length of desc in bytes, not necessarily
>> including
>> > + *                     trailing NUL.  It will be modified if desc is
>> invalid.
>> > + */
>> > +static const char*
>> > +ses_sanitize_elm_desc(const char *desc, uint16_t *len)
>> > +{
>> > +       const char *invalid =3D "<invalid>";
>> > +       int i;
>> > +
>> > +       for (i =3D 0; i < *len; i++) {
>> > +               if (desc[i] < 0x20 || desc[i] > 0x7e) {
>> > +                       *len =3D strlen(invalid);
>> > +                       return (invalid);
>> > +               } else if (desc[i] =3D=3D 0) {
>> > +                       break;
>> > +               }
>> > +       }
>> > +       return (desc);
>> > +}
>> > +
>> >  /**
>> >   * \brief Parse the descriptors for each object.
>> >   *
>> > @@ -2061,7 +2090,8 @@ ses_process_elm_descs(enc_softc_t *enc, struct
>> enc_fsm
>> >                 if (length > 0) {
>> >                         elmpriv =3D element->elm_private;
>> >                         elmpriv->descr_len =3D length;
>> > -                       elmpriv->descr =3D &buf[offset];
>> > +                       elmpriv->descr =3D
>> ses_sanitize_elm_desc(&buf[offset],
>> > +                           &elmpriv->descr_len);
>> >                 }
>> >
>> >                 /* skip over the descriptor itself */
>>
>>



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CANCZdfoVZOVUGQ1VPtEbaYkwPPADK-2K5pn_qpE5amEOQtY4VA>