From owner-svn-src-all@freebsd.org Wed Dec 11 17:43:10 2019 Return-Path: Delivered-To: svn-src-all@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id 029E51DC4B3 for ; Wed, 11 Dec 2019 17:43:10 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: from mail-qt1-x835.google.com (mail-qt1-x835.google.com [IPv6:2607:f8b0:4864:20::835]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (2048 bits) client-digest SHA256) (Client CN "smtp.gmail.com", Issuer "GTS CA 1O1" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 47Y45n2nw9z4Pvn for ; Wed, 11 Dec 2019 17:43:09 +0000 (UTC) (envelope-from wlosh@bsdimp.com) Received: by mail-qt1-x835.google.com with SMTP id p5so6966550qtq.12 for ; Wed, 11 Dec 2019 09:43:09 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=bsdimp-com.20150623.gappssmtp.com; s=20150623; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0kjCkC3Sz4nJQgKPk38/eAk2Ztxu19OyjuuGbiMBtjM=; b=sax3tPFSkn1j5vDnv1dkaHthi735kesR2nBsuDnMGm+1HXQmKuXvb53uPIkmGqyxQR ttMJl1pFJsEmXZ0t4I+bxyuUtFlkG/yipV86KMyx0lh6KDvfmACXkZyqNIccE3UCs3gI 6JFCB3yptGCl/5IOVFdQeC3jA6i1nYurGJrdo/0jcce+xmPZyeYBh7InCRiwYqxR6+da J/QHPU1SZDb8omWKRJ4ylRVJFMcib0gxwNR/1rvr1MtJmqFn3fwRp5seyDdeTG/4WHJJ UAcSzOtHlBSf0nwLV7O1H+AAduzN3OirgRakQxjyvo2yC8fNH+yB2zI4ZgFH47QORZdl PIww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0kjCkC3Sz4nJQgKPk38/eAk2Ztxu19OyjuuGbiMBtjM=; b=WrcyNUn1dJntl9k+g41meCymxMRn+9OFYRy9afX5VRbwqX94fLyNkxha+uNd2KaYgO GEN2uMeh9Y/7o1jOiNPPeHzru5r9gKNwpECvTZzBvTcxWlOO5L3eyYHJXcwjcgdVy2u6 6DXCAOSvtYengktiNYJZ2DUgfBY2RFfFVgmQG15jhluOPcRj3TQud0kPTuhRy8EDqD/y f37uHgHj5x8GYO4bvEJEt5MBSCTwf2GhDb2G36yjxoSBbqg7/A+cocF2nOivd0SMeFjO IQ4cEBkF5wqSfQY28EPHct+Arb1SgegwR8rDtn7Ts4wJgvDP35Z1/8Bc+FUosP5Wqf0y kHjg== X-Gm-Message-State: APjAAAVa0c6wmJBKhRJGlLwpnooyxSAi9t1L0zrfiiyd1asS+G/qhj39 IuTDSaf59Y6mL9vVBrUGgP38iJh3HLLRUhTXBSsfBQ== X-Google-Smtp-Source: APXvYqxx2JTP3E9Hht3uE/VLuMkD6RM98s//N1b1EWlbt4C1g3W2bKMvBptqlG7PKlBRn8ksX7QXF2oFEMoRLM2Zkmk= X-Received: by 2002:ac8:2cda:: with SMTP id 26mr3798043qtx.291.1576086188148; Wed, 11 Dec 2019 09:43:08 -0800 (PST) MIME-Version: 1.0 References: <201912060006.xB6066qR058963@repo.freebsd.org> <820BE55B-AE32-44E7-8AC7-245EF6F86F8B@samsco.org> In-Reply-To: From: Warner Losh Date: Wed, 11 Dec 2019 09:42:56 -0800 Message-ID: Subject: Re: svn commit: r355430 - head/sys/cam/scsi To: Alan Somers Cc: Scott Long , Steven Hartland , src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 47Y45n2nw9z4Pvn X-Spamd-Bar: --- Authentication-Results: mx1.freebsd.org; dkim=pass header.d=bsdimp-com.20150623.gappssmtp.com header.s=20150623 header.b=sax3tPFS; dmarc=none; spf=none (mx1.freebsd.org: domain of wlosh@bsdimp.com has no SPF policy when checking 2607:f8b0:4864:20::835) smtp.mailfrom=wlosh@bsdimp.com X-Spamd-Result: default: False [-3.70 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-1.00)[-0.999,0]; R_DKIM_ALLOW(-0.20)[bsdimp-com.20150623.gappssmtp.com:s=20150623]; FROM_HAS_DN(0.00)[]; NEURAL_HAM_LONG(-1.00)[-0.999,0]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; PREVIOUSLY_DELIVERED(0.00)[svn-src-all@freebsd.org]; DMARC_NA(0.00)[bsdimp.com]; URI_COUNT_ODD(1.00)[3]; RCPT_COUNT_FIVE(0.00)[6]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; DKIM_TRACE(0.00)[bsdimp-com.20150623.gappssmtp.com:+]; RCVD_IN_DNSWL_NONE(0.00)[5.3.8.0.0.0.0.0.0.0.0.0.0.0.0.0.0.2.0.0.4.6.8.4.0.b.8.f.7.0.6.2.list.dnswl.org : 127.0.5.0]; R_SPF_NA(0.00)[]; FORGED_SENDER(0.30)[imp@bsdimp.com,wlosh@bsdimp.com]; MIME_TRACE(0.00)[0:+,1:+,2:~]; IP_SCORE(-2.70)[ip: (-9.34), ipnet: 2607:f8b0::/32(-2.21), asn: 15169(-1.92), country: US(-0.05)]; ASN(0.00)[asn:15169, ipnet:2607:f8b0::/32, country:US]; FROM_NEQ_ENVFROM(0.00)[imp@bsdimp.com,wlosh@bsdimp.com]; RCVD_TLS_ALL(0.00)[]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Dec 2019 17:43:10 -0000 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 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 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 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 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 "". >> > >> > 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 ""; >> > + 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 */ >> >>