Skip to content

Commit

Permalink
USB: gadget: f_mass_storage: stale common->fsg value bug fix
Browse files Browse the repository at this point in the history 
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: Michal Nazarewicz <m.nazarewicz@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
  • Loading branch information
Michal Nazarewicz authored and Greg Kroah-Hartman committed Jun 30, 2010
1 parent e5fd39d commit b894f60
Showing 1 changed file with 61 additions and 99 deletions.
160 changes: 61 additions & 99 deletions drivers/usb/gadget/f_mass_storage.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,8 +321,8 @@ struct fsg_dev;
/* Data shared by all the FSG instances. */
struct fsg_common {
struct usb_gadget *gadget;
struct fsg_dev *fsg;
struct fsg_dev *prev_fsg;
struct fsg_dev *fsg, *new_fsg;
wait_queue_head_t fsg_wait;

/* filesem protects: backing files in use */
struct rw_semaphore filesem;
Expand Down Expand Up @@ -351,7 +351,6 @@ struct fsg_common {
enum fsg_state state; /* For exception handling */
unsigned int exception_req_tag;

u8 config, new_config;
enum data_direction data_dir;
u32 data_size;
u32 data_size_from_cmnd;
Expand Down Expand Up @@ -595,7 +594,7 @@ static int fsg_setup(struct usb_function *f,
u16 w_value = le16_to_cpu(ctrl->wValue);
u16 w_length = le16_to_cpu(ctrl->wLength);

if (!fsg->common->config)
if (!fsg_is_set(fsg->common))
return -EOPNOTSUPP;

switch (ctrl->bRequest) {
Expand Down Expand Up @@ -2303,24 +2302,20 @@ static int alloc_request(struct fsg_common *common, struct usb_ep *ep,
return -ENOMEM;
}

/*
* Reset interface setting and re-init endpoint state (toggle etc).
* 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)
/* Reset interface setting and re-init endpoint state (toggle etc). */
static int do_set_interface(struct fsg_common *common, struct fsg_dev *new_fsg)
{
int rc = 0;
int i;
const struct usb_endpoint_descriptor *d;
const struct usb_endpoint_descriptor *d;
struct fsg_dev *fsg;
int i, rc = 0;

if (common->running)
DBG(common, "reset interface\n");

reset:
/* Deallocate the requests */
if (common->prev_fsg) {
struct fsg_dev *fsg = common->prev_fsg;
if (common->fsg) {
fsg = common->fsg;

for (i = 0; i < FSG_NUM_BUFFERS; ++i) {
struct fsg_buffhd *bh = &common->buffhds[i];
Expand All @@ -2345,88 +2340,53 @@ static int do_set_interface(struct fsg_common *common, int altsetting)
fsg->bulk_out_enabled = 0;
}

common->prev_fsg = 0;
common->fsg = NULL;
wake_up(&common->fsg_wait);
}

common->running = 0;
if (altsetting < 0 || rc != 0)
if (!new_fsg || rc)
return rc;

DBG(common, "set interface %d\n", altsetting);
common->fsg = new_fsg;
fsg = common->fsg;

if (fsg_is_set(common)) {
struct fsg_dev *fsg = common->fsg;
common->prev_fsg = common->fsg;
/* Enable the endpoints */
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)
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 */
d = fsg_ep_desc(common->gadget,
&fsg_fs_bulk_in_desc, &fsg_hs_bulk_in_desc);
rc = enable_endpoint(common, fsg->bulk_in, d);
rc = alloc_request(common, fsg->bulk_in, &bh->inreq);
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);
rc = alloc_request(common, fsg->bulk_out, &bh->outreq);
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];

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;
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;
}
}


/*
* Change our operational configuration. This code must agree with the code
* 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 */
}
common->running = 1;
for (i = 0; i < common->nluns; ++i)
common->luns[i].unit_attention_data = SS_RESET_OCCURRED;
return rc;
}

Expand All @@ -2437,19 +2397,15 @@ 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)
{
struct fsg_dev *fsg = fsg_from_func(f);
fsg->common->prev_fsg = fsg->common->fsg;
fsg->common->fsg = fsg;
fsg->common->new_config = 1;
fsg->common->new_fsg = fsg;
raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
return 0;
}

static void fsg_disable(struct usb_function *f)
{
struct fsg_dev *fsg = fsg_from_func(f);
fsg->common->prev_fsg = fsg->common->fsg;
fsg->common->fsg = fsg;
fsg->common->new_config = 0;
fsg->common->new_fsg = NULL;
raise_exception(fsg->common, FSG_STATE_CONFIG_CHANGE);
}

Expand All @@ -2459,19 +2415,17 @@ static void fsg_disable(struct usb_function *f)
static void handle_exception(struct fsg_common *common)
{
siginfo_t info;
int sig;
int i;
struct fsg_buffhd *bh;
enum fsg_state old_state;
u8 new_config;
struct fsg_lun *curlun;
unsigned int exception_req_tag;
int rc;

/* Clear the existing signals. Anything but SIGUSR1 is converted
* into a high-priority EXIT exception. */
for (;;) {
sig = dequeue_signal_lock(current, &current->blocked, &info);
int sig =
dequeue_signal_lock(current, &current->blocked, &info);
if (!sig)
break;
if (sig != SIGUSR1) {
Expand All @@ -2482,7 +2436,7 @@ static void handle_exception(struct fsg_common *common)
}

/* Cancel all the pending transfers */
if (fsg_is_set(common)) {
if (likely(common->fsg)) {
for (i = 0; i < FSG_NUM_BUFFERS; ++i) {
bh = &common->buffhds[i];
if (bh->inreq_busy)
Expand Down Expand Up @@ -2523,7 +2477,6 @@ static void handle_exception(struct fsg_common *common)
common->next_buffhd_to_fill = &common->buffhds[0];
common->next_buffhd_to_drain = &common->buffhds[0];
exception_req_tag = common->exception_req_tag;
new_config = common->new_config;
old_state = common->state;

if (old_state == FSG_STATE_ABORT_BULK_OUT)
Expand Down Expand Up @@ -2573,12 +2526,12 @@ static void handle_exception(struct fsg_common *common)
break;

case FSG_STATE_CONFIG_CHANGE:
rc = do_set_config(common, new_config);
do_set_interface(common, common->new_fsg);
break;

case FSG_STATE_EXIT:
case FSG_STATE_TERMINATED:
do_set_config(common, 0); /* Free resources */
do_set_interface(common, NULL); /* Free resources */
spin_lock_irq(&common->lock);
common->state = FSG_STATE_TERMINATED; /* Stop the thread */
spin_unlock_irq(&common->lock);
Expand Down Expand Up @@ -2863,6 +2816,7 @@ static struct fsg_common *fsg_common_init(struct fsg_common *common,
goto error_release;
}
init_completion(&common->thread_notifier);
init_waitqueue_head(&common->fsg_wait);
#undef OR


Expand Down Expand Up @@ -2957,9 +2911,17 @@ static void fsg_common_release(struct kref *ref)
static void fsg_unbind(struct usb_configuration *c, struct usb_function *f)
{
struct fsg_dev *fsg = fsg_from_func(f);
struct fsg_common *common = fsg->common;

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.hs_descriptors);
kfree(fsg);
Expand Down

0 comments on commit b894f60

Please sign in to comment.