On 07/08/2015 04:00 PM, Michal Nazarewicz wrote:
On Tue, Jul 07 2015, Krzysztof Opasiak wrote:
This patch replace dynamicly allocated luns array with static one.
This simplifies the code of mass storage function and modules.
It also fix issue with reporting wrong number of LUNs in GET_MAX_LUN
request. According to MS spec we should return the max index of valid
LUN, not the number of luns - 1.
This is no longer true, is it? Why my fix this bug has been solved, no?
As such, this should not go to stable. Or am I missing something?
First of all please excuse me my late response but I were out of office.
Unfortunately it's still true. Your fix solved this bug partially. Now
we report nluns - 1 instead of FSG_LUN_MAX but in case of not contiguous
luns it is not enough.
Let's consider mass storage function with luns 0 1 3 5. nluns == 4 so in
GET_MAX_LUN we will return 3 (nluns - 1). Some hosts (in particular
Windows) iterate over all luns up to value returned from this request.
This means that host will ask about LUNs 0 1 2 3. LUN 2 is invalid and
will be skipped but he will never ask about luns 4 and 5 what makes LUN
5 unavailable.
Standard says that GET_MAX_LUNS should return index of last valid LUN
which in this case is 5 not 3 and this is what this patch does.
Reported-by: David Fisher <david.fisher1@xxxxxxxxxxxx>
Signed-off-by: Krzysztof Opasiak <k.opasiak@xxxxxxxxxxx>
Acked-by: Michal Nazarewicz <mina86@xxxxxxxxxx>
CC: stable@xxxxxxxxxxxxxxx # 3.13+
@@ -490,6 +489,16 @@ static void bulk_out_complete(struct usb_ep *ep, struct usb_request *req)
spin_unlock(&common->lock);
}
+static int _fsg_common_get_max_lun(struct fsg_common *common)
Perhaps even ‘unsigned’.
As 0 is a valid LUN index this function returns -1 if there is no LUNs
at all.
+{
+ int i = ARRAY_SIZE(common->luns) - 1;
Ditto. This applies in other places of the patch so I won’t keep
repeating it.
+
+ while (i >= 0 && !common->luns[i])
+ --i;
+
+ return i;
+}
+
static int fsg_setup(struct usb_function *f,
const struct usb_ctrlrequest *ctrl)
{
@@ -2552,12 +2562,11 @@ static int fsg_main_thread(void *common_)
if (!common->ops || !common->ops->thread_exits
|| common->ops->thread_exits(common) < 0) {
- struct fsg_lun **curlun_it = common->luns;
- unsigned i = common->nluns;
+ int i;
down_write(&common->filesem);
- for (; i--; ++curlun_it) {
- struct fsg_lun *curlun = *curlun_it;
+ for (i = 0; i < ARRAY_SIZE(common->luns); --i) {
++i
+ struct fsg_lun *curlun = common->luns[i];
if (!curlun || !fsg_lun_is_open(curlun))
continue;
--
Krzysztof Opasiak
Samsung R&D Institute Poland
Samsung Electronics
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html