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>