All of lore.kernel.org
 help / color / mirror / Atom feed
From: John Keeping <john@metanate.com>
To: Felipe Balbi <balbi@kernel.org>
Cc: linux-usb@vger.kernel.org, linux-kernel@vger.kernel.org,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Vincent Pelletier <plr.vincent@gmail.com>,
	Jim Lin <jilin@nvidia.com>
Subject: [PATCH v2] usb: f_fs: Force Reserved1=1 in OS_DESC_EXT_COMPAT
Date: Mon, 27 Nov 2017 18:15:40 +0000	[thread overview]
Message-ID: <20171127181540.65aa748a.john@metanate.com> (raw)
In-Reply-To: <87vahwklab.fsf@linux.intel.com>

The specification says that the Reserved1 field in OS_DESC_EXT_COMPAT
must have the value "1", but when this feature was first implemented we
rejected any non-zero values.

This was adjusted to accept all non-zero values (while now rejecting
zero) in commit 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on
reserved1 of OS_DESC_EXT_COMPAT"), but that breaks any userspace
programs that worked previously by returning EINVAL when Reserved1 == 0
which was previously the only value that succeeded!

If we just set the field to "1" ourselves, both old and new userspace
programs continue to work correctly and, as a bonus, old programs are
now compliant with the specification without having to fix anything
themselves.

Fixes: 53642399aa71 ("usb: gadget: f_fs: Fix wrong check on reserved1 of OS_DESC_EXT_COMPAT")
Cc: stable@vger.kernel.org
Signed-off-by: John Keeping <john@metanate.com>
---
 drivers/usb/gadget/function/f_fs.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

On Mon, 27 Nov 2017 13:34:20 +0200, Felipe Balbi wrote:

> exactly like that. Care to send as a formal patch so I can apply?

Here it is.

diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 97ea059a7aa4..7537cc1a7d79 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -2282,9 +2282,18 @@ static int __ffs_data_do_os_desc(enum ffs_os_desc_type type,
 		int i;
 
 		if (len < sizeof(*d) ||
-		    d->bFirstInterfaceNumber >= ffs->interfaces_count ||
-		    !d->Reserved1)
+		    d->bFirstInterfaceNumber >= ffs->interfaces_count)
 			return -EINVAL;
+		if (d->Reserved1 != 1) {
+			/*
+			 * According to the spec, Reserved1 must be set to 1
+			 * but older kernels incorrectly rejected non-zero
+			 * values.  We fix it here to avoid returning EINVAL
+			 * in response to values we used to accept.
+			 */
+			pr_debug("usb_ext_compat_desc::Reserved1 forced to 1\n");
+			d->Reserved1 = 1;
+		}
 		for (i = 0; i < ARRAY_SIZE(d->Reserved2); ++i)
 			if (d->Reserved2[i])
 				return -EINVAL;
-- 
2.15.0

      reply	other threads:[~2017-11-27 18:16 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-11-09 16:34 [PATCH] usb: f_fs: Drop check on Reserved1 field on OS_DESC_EXT_COMPAT John Keeping
2017-11-10 10:40 ` Felipe Balbi
2017-11-10 18:34   ` John Keeping
2017-11-13 10:57     ` Felipe Balbi
2017-11-13 18:19       ` John Keeping
2017-11-27 11:34         ` Felipe Balbi
2017-11-27 18:15           ` John Keeping [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171127181540.65aa748a.john@metanate.com \
    --to=john@metanate.com \
    --cc=balbi@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=jilin@nvidia.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-usb@vger.kernel.org \
    --cc=plr.vincent@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.