Commit b894f60a authored by Michal Nazarewicz's avatar Michal Nazarewicz Committed by Greg Kroah-Hartman

USB: gadget: f_mass_storage: stale common->fsg value bug fix

On fsg_unbind the common->fsg pointer was not NULLed if the
unbound fsg_dev instance was the current one.  As an effect,
the incorrect pointer was preserved in all further operations
which caused do_set_interface to reference an invalid region.

This commit fixes this by raising an exception in fsg_bind
which will change the common->fsg pointer.  This also requires
an wait queue so that the thread in fsg_bind can wait till the
worker thread handles the exception.

This commit removes also a config and new_config fields of
fsg_common as they are no longer needed since fsg can be
used to determine whether function is active or not.

Moreover, this commit removes possible race condition where
the fsg field was modified in both the worker thread and
form various other contexts.  This is fixed by replacing
prev_fsg with new_fsg.  At this point, fsg is assigned only
in worker thread.
Signed-off-by: default avatarMichal Nazarewicz <m.nazarewicz@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
parent e5fd39d9
...@@ -321,8 +321,8 @@ struct fsg_dev; ...@@ -321,8 +321,8 @@ struct fsg_dev;
/* Data shared by all the FSG instances. */ /* Data shared by all the FSG instances. */
struct fsg_common { struct fsg_common {
struct usb_gadget *gadget; struct usb_gadget *gadget;
struct fsg_dev *fsg; struct fsg_dev *fsg, *new_fsg;
struct fsg_dev *prev_fsg; wait_queue_head_t fsg_wait;
/* filesem protects: backing files in use */ /* filesem protects: backing files in use */
struct rw_semaphore filesem; struct rw_semaphore filesem;
...@@ -351,7 +351,6 @@ struct fsg_common { ...@@ -351,7 +351,6 @@ struct fsg_common {
enum fsg_state state; /* For exception handling */ enum fsg_state state; /* For exception handling */
unsigned int exception_req_tag; unsigned int exception_req_tag;
u8 config, new_config;
enum data_direction data_dir; enum data_direction data_dir;
u32 data_size; u32 data_size;
u32 data_size_from_cmnd; u32 data_size_from_cmnd;
...@@ -595,7 +594,7 @@ static int fsg_setup(struct usb_function *f, ...@@ -595,7 +594,7 @@ static int fsg_setup(struct usb_function *f,
u16 w_value = le16_to_cpu(ctrl->wValue); u16 w_value = le16_to_cpu(ctrl->wValue);
u16 w_length = le16_to_cpu(ctrl->wLength); u16 w_length = le16_to_cpu(ctrl->wLength);
if (!fsg->common->config) if (!fsg_is_set(fsg->common))
return -EOPNOTSUPP; return -EOPNOTSUPP;
switch (ctrl->bRequest) { switch (ctrl->bRequest) {
...@@ -2303,24 +2302,20 @@ static int alloc_request(struct fsg_common *common, struct usb_ep *ep, ...@@ -2303,24 +2302,20 @@ static int alloc_request(struct fsg_common *common, struct usb_ep *ep,
return -ENOMEM; return -ENOMEM;
} }
/* /* Reset interface setting and re-init endpoint state (toggle etc). */
* Reset interface setting and re-init endpoint state (toggle etc). static int do_set_interface(struct fsg_common *common, struct fsg_dev *new_fsg)
* Call with altsetting < 0 to disable the interface. The only other
* available altsetting is 0, which enables the interface.
*/
static int do_set_interface(struct fsg_common *common, int altsetting)
{ {
int rc = 0; const struct usb_endpoint_descriptor *d;
int i; struct fsg_dev *fsg;
const struct usb_endpoint_descriptor *d; int i, rc = 0;
if (common->running) if (common->running)
DBG(common, "reset interface\n"); DBG(common, "reset interface\n");
reset: reset:
/* Deallocate the requests */ /* Deallocate the requests */
if (common->prev_fsg) { if (common->fsg) {
struct fsg_dev *fsg = common->prev_fsg; fsg = common->fsg;
for (i = 0; i < FSG_NUM_BUFFERS; ++i) { for (i = 0; i < FSG_NUM_BUFFERS; ++i) {
struct fsg_buffhd *bh = &common->buffhds[i]; struct fsg_buffhd *bh = &common->buffhds[i];
...@@ -2345,88 +2340,53 @@ static int do_set_interface(struct fsg_common *common, int altsetting) ...@@ -2345,88 +2340,53 @@ static int do_set_interface(struct fsg_common *common, int altsetting)
fsg->bulk_out_enabled = 0; fsg->bulk_out_enabled = 0;
} }
common->prev_fsg = 0; common->fsg = NULL;
wake_up(&common->fsg_wait);
} }
common->running = 0; common->running = 0;
if (altsetting < 0 || rc != 0) if (!new_fsg || rc)
return rc; return rc;
DBG(common, "set interface %d\n", altsetting); common->fsg = new_fsg;
fsg = common->fsg;
if (fsg_is_set(common)) { /* Enable the endpoints */
struct fsg_dev *fsg = common->fsg; d = fsg_ep_desc(common->gadget,
common->prev_fsg = common->fsg; &fsg_fs_bulk_in_desc, &fsg_hs_bulk_in_desc);
rc = enable_endpoint(common, fsg->bulk_in, d);
if (rc)
goto reset;
fsg->bulk_in_enabled = 1;
d = fsg_ep_desc(common->gadget,
&fsg_fs_bulk_out_desc, &fsg_hs_bulk_out_desc);
rc = enable_endpoint(common, fsg->bulk_out, d);
if (rc)
goto reset;
fsg->bulk_out_enabled = 1;
common->bulk_out_maxpacket = le16_to_cpu(d->wMaxPacketSize);
clear_bit(IGNORE_BULK_OUT, &fsg->atomic_bitflags);
/* Allocate the requests */
for (i = 0; i < FSG_NUM_BUFFERS; ++i) {
struct fsg_buffhd *bh = &common->buffhds[i];
/* Enable the endpoints */ rc = alloc_request(common, fsg->bulk_in, &bh->inreq);
d = fsg_ep_desc(common->gadget,
&fsg_fs_bulk_in_desc, &fsg_hs_bulk_in_desc);
rc = enable_endpoint(common, fsg->bulk_in, d);
if (rc) if (rc)
goto reset; goto reset;
fsg->bulk_in_enabled = 1; rc = alloc_request(common, fsg->bulk_out, &bh->outreq);
d = fsg_ep_desc(common->gadget,
&fsg_fs_bulk_out_desc, &fsg_hs_bulk_out_desc);
rc = enable_endpoint(common, fsg->bulk_out, d);
if (rc) if (rc)
goto reset; goto reset;
fsg->bulk_out_enabled = 1; bh->inreq->buf = bh->outreq->buf = bh->buf;
common->bulk_out_maxpacket = le16_to_cpu(d->wMaxPacketSize); bh->inreq->context = bh->outreq->context = bh;
clear_bit(IGNORE_BULK_OUT, &fsg->atomic_bitflags); bh->inreq->complete = bulk_in_complete;
bh->outreq->complete = bulk_out_complete;
/* Allocate the requests */
for (i = 0; i < FSG_NUM_BUFFERS; ++i) {
struct fsg_buffhd *bh = &common->buffhds[i];
rc = alloc_request(common, fsg->bulk_in, &bh->inreq);
if (rc)
goto reset;
rc = alloc_request(common, fsg->bulk_out, &bh->outreq);
if (rc)
goto reset;
bh->inreq->buf = bh->outreq->buf = bh->buf;
bh->inreq->context = bh->outreq->context = bh;
bh->inreq->complete = bulk_in_complete;
bh->outreq->complete = bulk_out_complete;
}
common->running = 1;
for (i = 0; i < common->nluns; ++i)
common->luns[i].unit_attention_data = SS_RESET_OCCURRED;
return rc;
} else {
return -EIO;
} }
}
common->running = 1;
/* for (i = 0; i < common->nluns; ++i)
* Change our operational configuration. This code must agree with the code common->luns[i].unit_attention_data = SS_RESET_OCCURRED;
* that returns config descriptors, and with interface altsetting code.
*
* It's also responsible for power management interactions. Some
* configurations might not work with our current power sources.
* For now we just assume the gadget is always self-powered.
*/
static int do_set_config(struct fsg_common *common, u8 new_config)
{
int rc = 0;
/* Disable the single interface */
if (common->config != 0) {
DBG(common, "reset config\n");
common->config = 0;
rc = do_set_interface(common, -1);
}
/* Enable the interface */
if (new_config != 0) {
common->config = new_config;
rc = do_set_interface(common, 0);
if (rc != 0)
common->config = 0; /* Reset on errors */
}
return rc; return rc;
} }
...@@ -2437,9 +2397,7 @@ static int do_set_config(struct fsg_common *common, u8 new_config) ...@@ -2437,9 +2397,7 @@ static int do_set_config(struct fsg_common *common, u8 new_config)
static int fsg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) static int fsg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
{ {
struct fsg_dev *fsg = fsg_from_func(f); struct fsg_dev *fsg = fsg_from_func(f);
fsg->common->prev_fsg = fsg->common->fsg; fsg->common->new_fsg = fsg;
fsg->common->fsg = fsg;
fsg->common->new_config = 1;
raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE); raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
return 0; return 0;
} }
...@@ -2447,9 +2405,7 @@ static int fsg_set_alt(struct usb_function *f, unsigned intf, unsigned alt) ...@@ -2447,9 +2405,7 @@ static int fsg_set_alt(struct usb_function *f, unsigned intf, unsigned alt)
static void fsg_disable(struct usb_function *f) static void fsg_disable(struct usb_function *f)
{ {
struct fsg_dev *fsg = fsg_from_func(f); struct fsg_dev *fsg = fsg_from_func(f);
fsg->common->prev_fsg = fsg->common->fsg; fsg->common->new_fsg = NULL;
fsg->common->fsg = fsg;
fsg->common->new_config = 0;
raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE); raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
} }
...@@ -2459,19 +2415,17 @@ static void fsg_disable(struct usb_function *f) ...@@ -2459,19 +2415,17 @@ static void fsg_disable(struct usb_function *f)
static void handle_exception(struct fsg_common *common) static void handle_exception(struct fsg_common *common)
{ {
siginfo_t info; siginfo_t info;
int sig;
int i; int i;
struct fsg_buffhd *bh; struct fsg_buffhd *bh;
enum fsg_state old_state; enum fsg_state old_state;
u8 new_config;
struct fsg_lun *curlun; struct fsg_lun *curlun;
unsigned int exception_req_tag; unsigned int exception_req_tag;
int rc;
/* Clear the existing signals. Anything but SIGUSR1 is converted /* Clear the existing signals. Anything but SIGUSR1 is converted
* into a high-priority EXIT exception. */ * into a high-priority EXIT exception. */
for (;;) { for (;;) {
sig = dequeue_signal_lock(current, &current->blocked, &info); int sig =
dequeue_signal_lock(current, &current->blocked, &info);
if (!sig) if (!sig)
break; break;
if (sig != SIGUSR1) { if (sig != SIGUSR1) {
...@@ -2482,7 +2436,7 @@ static void handle_exception(struct fsg_common *common) ...@@ -2482,7 +2436,7 @@ static void handle_exception(struct fsg_common *common)
} }
/* Cancel all the pending transfers */ /* Cancel all the pending transfers */
if (fsg_is_set(common)) { if (likely(common->fsg)) {
for (i = 0; i < FSG_NUM_BUFFERS; ++i) { for (i = 0; i < FSG_NUM_BUFFERS; ++i) {
bh = &common->buffhds[i]; bh = &common->buffhds[i];
if (bh->inreq_busy) if (bh->inreq_busy)
...@@ -2523,7 +2477,6 @@ static void handle_exception(struct fsg_common *common) ...@@ -2523,7 +2477,6 @@ static void handle_exception(struct fsg_common *common)
common->next_buffhd_to_fill = &common->buffhds[0]; common->next_buffhd_to_fill = &common->buffhds[0];
common->next_buffhd_to_drain = &common->buffhds[0]; common->next_buffhd_to_drain = &common->buffhds[0];
exception_req_tag = common->exception_req_tag; exception_req_tag = common->exception_req_tag;
new_config = common->new_config;
old_state = common->state; old_state = common->state;
if (old_state == FSG_STATE_ABORT_BULK_OUT) if (old_state == FSG_STATE_ABORT_BULK_OUT)
...@@ -2573,12 +2526,12 @@ static void handle_exception(struct fsg_common *common) ...@@ -2573,12 +2526,12 @@ static void handle_exception(struct fsg_common *common)
break; break;
case FSG_STATE_CONFIG_CHANGE: case FSG_STATE_CONFIG_CHANGE:
rc = do_set_config(common, new_config); do_set_interface(common, common->new_fsg);
break; break;
case FSG_STATE_EXIT: case FSG_STATE_EXIT:
case FSG_STATE_TERMINATED: case FSG_STATE_TERMINATED:
do_set_config(common, 0); /* Free resources */ do_set_interface(common, NULL); /* Free resources */
spin_lock_irq(&common->lock); spin_lock_irq(&common->lock);
common->state = FSG_STATE_TERMINATED; /* Stop the thread */ common->state = FSG_STATE_TERMINATED; /* Stop the thread */
spin_unlock_irq(&common->lock); spin_unlock_irq(&common->lock);
...@@ -2863,6 +2816,7 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common, ...@@ -2863,6 +2816,7 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common,
goto error_release; goto error_release;
} }
init_completion(&common->thread_notifier); init_completion(&common->thread_notifier);
init_waitqueue_head(&common->fsg_wait);
#undef OR #undef OR
...@@ -2957,9 +2911,17 @@ static void fsg_common_release(struct kref *ref) ...@@ -2957,9 +2911,17 @@ static void fsg_common_release(struct kref *ref)
static void fsg_unbind(struct usb_configuration *c, struct usb_function *f) static void fsg_unbind(struct usb_configuration *c, struct usb_function *f)
{ {
struct fsg_dev *fsg = fsg_from_func(f); struct fsg_dev *fsg = fsg_from_func(f);
struct fsg_common *common = fsg->common;
DBG(fsg, "unbind\n"); DBG(fsg, "unbind\n");
fsg_common_put(fsg->common); if (fsg->common->fsg == fsg) {
fsg->common->new_fsg = NULL;
raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
/* FIXME: make interruptible or killable somehow? */
wait_event(common->fsg_wait, common->fsg != fsg);
}
fsg_common_put(common);
usb_free_descriptors(fsg->function.descriptors); usb_free_descriptors(fsg->function.descriptors);
usb_free_descriptors(fsg->function.hs_descriptors); usb_free_descriptors(fsg->function.hs_descriptors);
kfree(fsg); kfree(fsg);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment