From owner-svn-src-head@freebsd.org Wed Dec 11 15:42:25 2019 Return-Path: Delivered-To: svn-src-head@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 846871D9334; Wed, 11 Dec 2019 15:42:25 +0000 (UTC) (envelope-from asomers@gmail.com) Received: from mail-oi1-f179.google.com (mail-oi1-f179.google.com [209.85.167.179]) (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 47Y1QS4C94z4Ffb; Wed, 11 Dec 2019 15:42:24 +0000 (UTC) (envelope-from asomers@gmail.com) Received: by mail-oi1-f179.google.com with SMTP id x195so13634371oix.4; Wed, 11 Dec 2019 07:42:24 -0800 (PST) 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=rh0CPYvcRm0t7W/VYVpPksJTIht+Ik7nMcU5rasv+5g=; b=uOPtUdHTq2yYFGRGk88KRUQPhtqm0C90JUcdFwAJry1ZvLNwFx+VCgt+iNNyLGgTgW FBxpHdLynnd/TFp0Rl0TLuZLQDssFmsOCO9/L2g+lQIZV4d6BseedA3eBc4JAWn46orU PkiIvbIr5RshNAJ0YKYaYcz5oJ3swKT/hppeeUDWlzCHeMnIpif0zJctQZuYC7TehlWS jMcnyiBnQ0EwGeFhyEKhYJoltvdUceQNxWTuN+LXZpnNg5hN//8EOKXY0CS7P0H2HkQP E0ojTuyxL4H/A0osvQl3LcotYjxbw3i72MzFMHeWAo093zyixecvWQ9B23DaXjdRWFvT LNdQ== X-Gm-Message-State: APjAAAVXumeqsgrrlnfbxNWxRk75FUNu56487Wgw8ns1LCDsMKTRpxJb INpRncHjxs87JvpG65F4JWwOa8/cpt1uDJzmcMYADw== X-Google-Smtp-Source: APXvYqwB4pETbFbboYhbZxMzLTZoF1DkSe7E2X/3C782ck9a6gzUXNOf5TUz4RNxm7KOKrbpofVPJg0RycQiPedCws0= X-Received: by 2002:aca:b2c5:: with SMTP id b188mr3091784oif.55.1576078943429; Wed, 11 Dec 2019 07:42:23 -0800 (PST) MIME-Version: 1.0 References: <201912060006.xB6066qR058963@repo.freebsd.org> In-Reply-To: From: Alan Somers Date: Wed, 11 Dec 2019 08:42:11 -0700 Message-ID: Subject: Re: svn commit: r355430 - head/sys/cam/scsi To: Steven Hartland Cc: src-committers , svn-src-all , svn-src-head X-Rspamd-Queue-Id: 47Y1QS4C94z4Ffb X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org; dkim=none; dmarc=none; spf=pass (mx1.freebsd.org: domain of asomers@gmail.com designates 209.85.167.179 as permitted sender) smtp.mailfrom=asomers@gmail.com X-Spamd-Result: default: False [-2.04 / 15.00]; ARC_NA(0.00)[]; NEURAL_HAM_MEDIUM(-0.99)[-0.989,0]; RCVD_TLS_ALL(0.00)[]; FROM_HAS_DN(0.00)[]; RCPT_COUNT_THREE(0.00)[4]; R_SPF_ALLOW(-0.20)[+ip4:209.85.128.0/17:c]; NEURAL_HAM_LONG(-0.98)[-0.978,0]; MIME_GOOD(-0.10)[multipart/alternative,text/plain]; DMARC_NA(0.00)[freebsd.org]; URI_COUNT_ODD(1.00)[3]; TO_MATCH_ENVRCPT_SOME(0.00)[]; TO_DN_ALL(0.00)[]; RCVD_IN_DNSWL_NONE(0.00)[179.167.85.209.list.dnswl.org : 127.0.5.0]; IP_SCORE(-1.07)[ip: (-0.27), ipnet: 209.85.128.0/17(-3.14), asn: 15169(-1.92), country: US(-0.05)]; FORGED_SENDER(0.30)[asomers@freebsd.org,asomers@gmail.com]; RWL_MAILSPIKE_POSSIBLE(0.00)[179.167.85.209.rep.mailspike.net : 127.0.0.17]; MIME_TRACE(0.00)[0:+,1:+,2:~]; R_DKIM_NA(0.00)[]; FREEMAIL_ENVFROM(0.00)[gmail.com]; ASN(0.00)[asn:15169, ipnet:209.85.128.0/17, country:US]; FROM_NEQ_ENVFROM(0.00)[asomers@freebsd.org,asomers@gmail.com]; RCVD_COUNT_TWO(0.00)[2] Content-Type: text/plain; charset="UTF-8" X-Content-Filtered-By: Mailman/MimeDel 2.1.29 X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Dec 2019 15:42:25 -0000 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 that 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 in >> 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 >> >> ============================================================================== >> --- 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 = ""; >> + int i; >> + >> + for (i = 0; i < *len; i++) { >> + if (desc[i] < 0x20 || desc[i] > 0x7e) { >> + *len = strlen(invalid); >> + return (invalid); >> + } else if (desc[i] == 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 = element->elm_private; >> elmpriv->descr_len = length; >> - elmpriv->descr = &buf[offset]; >> + elmpriv->descr = >> ses_sanitize_elm_desc(&buf[offset], >> + &elmpriv->descr_len); >> } >> >> /* skip over the descriptor itself */ >> >