diff options
author | Alan Stern <stern@rowland.harvard.edu> | 2009-04-16 15:37:28 -0400 |
---|---|---|
committer | Greg Kroah-Hartman <gregkh@suse.de> | 2009-04-23 17:15:28 -0400 |
commit | 82a10a81c853be3859b3d222db0f372ee8d2eaa2 (patch) | |
tree | 717c7bdc51f849982a850b6fbbdbf1af36cf88d0 | |
parent | c065c60e83c006611caed23d1320450fcd709398 (diff) |
USB: g_file_storage: fix use-after-free bug when closing files
This patch (as1231) fixes a use-after-free bug in g_file_storage. A
device's name may not be available after the device is unregistered,
even if the device structure itself is still allocated. Since
close_backing_file() prints a LUN's name for debugging, it shouldn't
be called after the LUN has been unregistered.
That whole area needed to be cleaned up; the backing files were
getting closed in a couple of different places. The patch fixes
things so that they get closed in just one place, as part of the
unbind procedure, immediately before the LUN is unregistered.
Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
-rw-r--r-- | drivers/usb/gadget/file_storage.c | 20 |
1 files changed, 4 insertions, 16 deletions
diff --git a/drivers/usb/gadget/file_storage.c b/drivers/usb/gadget/file_storage.c index 5c030b080d4c..381a53b3e11c 100644 --- a/drivers/usb/gadget/file_storage.c +++ b/drivers/usb/gadget/file_storage.c | |||
@@ -738,7 +738,6 @@ static struct fsg_dev *the_fsg; | |||
738 | static struct usb_gadget_driver fsg_driver; | 738 | static struct usb_gadget_driver fsg_driver; |
739 | 739 | ||
740 | static void close_backing_file(struct lun *curlun); | 740 | static void close_backing_file(struct lun *curlun); |
741 | static void close_all_backing_files(struct fsg_dev *fsg); | ||
742 | 741 | ||
743 | 742 | ||
744 | /*-------------------------------------------------------------------------*/ | 743 | /*-------------------------------------------------------------------------*/ |
@@ -3593,12 +3592,10 @@ static int fsg_main_thread(void *fsg_) | |||
3593 | fsg->thread_task = NULL; | 3592 | fsg->thread_task = NULL; |
3594 | spin_unlock_irq(&fsg->lock); | 3593 | spin_unlock_irq(&fsg->lock); |
3595 | 3594 | ||
3596 | /* In case we are exiting because of a signal, unregister the | 3595 | /* If we are exiting because of a signal, unregister the |
3597 | * gadget driver and close the backing file. */ | 3596 | * gadget driver. */ |
3598 | if (test_and_clear_bit(REGISTERED, &fsg->atomic_bitflags)) { | 3597 | if (test_and_clear_bit(REGISTERED, &fsg->atomic_bitflags)) |
3599 | usb_gadget_unregister_driver(&fsg_driver); | 3598 | usb_gadget_unregister_driver(&fsg_driver); |
3600 | close_all_backing_files(fsg); | ||
3601 | } | ||
3602 | 3599 | ||
3603 | /* Let the unbind and cleanup routines know the thread has exited */ | 3600 | /* Let the unbind and cleanup routines know the thread has exited */ |
3604 | complete_and_exit(&fsg->thread_notifier, 0); | 3601 | complete_and_exit(&fsg->thread_notifier, 0); |
@@ -3703,14 +3700,6 @@ static void close_backing_file(struct lun *curlun) | |||
3703 | } | 3700 | } |
3704 | } | 3701 | } |
3705 | 3702 | ||
3706 | static void close_all_backing_files(struct fsg_dev *fsg) | ||
3707 | { | ||
3708 | int i; | ||
3709 | |||
3710 | for (i = 0; i < fsg->nluns; ++i) | ||
3711 | close_backing_file(&fsg->luns[i]); | ||
3712 | } | ||
3713 | |||
3714 | 3703 | ||
3715 | static ssize_t show_ro(struct device *dev, struct device_attribute *attr, char *buf) | 3704 | static ssize_t show_ro(struct device *dev, struct device_attribute *attr, char *buf) |
3716 | { | 3705 | { |
@@ -3845,6 +3834,7 @@ static void /* __init_or_exit */ fsg_unbind(struct usb_gadget *gadget) | |||
3845 | if (curlun->registered) { | 3834 | if (curlun->registered) { |
3846 | device_remove_file(&curlun->dev, &dev_attr_ro); | 3835 | device_remove_file(&curlun->dev, &dev_attr_ro); |
3847 | device_remove_file(&curlun->dev, &dev_attr_file); | 3836 | device_remove_file(&curlun->dev, &dev_attr_file); |
3837 | close_backing_file(curlun); | ||
3848 | device_unregister(&curlun->dev); | 3838 | device_unregister(&curlun->dev); |
3849 | curlun->registered = 0; | 3839 | curlun->registered = 0; |
3850 | } | 3840 | } |
@@ -4190,7 +4180,6 @@ autoconf_fail: | |||
4190 | out: | 4180 | out: |
4191 | fsg->state = FSG_STATE_TERMINATED; // The thread is dead | 4181 | fsg->state = FSG_STATE_TERMINATED; // The thread is dead |
4192 | fsg_unbind(gadget); | 4182 | fsg_unbind(gadget); |
4193 | close_all_backing_files(fsg); | ||
4194 | complete(&fsg->thread_notifier); | 4183 | complete(&fsg->thread_notifier); |
4195 | return rc; | 4184 | return rc; |
4196 | } | 4185 | } |
@@ -4284,7 +4273,6 @@ static void __exit fsg_cleanup(void) | |||
4284 | /* Wait for the thread to finish up */ | 4273 | /* Wait for the thread to finish up */ |
4285 | wait_for_completion(&fsg->thread_notifier); | 4274 | wait_for_completion(&fsg->thread_notifier); |
4286 | 4275 | ||
4287 | close_all_backing_files(fsg); | ||
4288 | kref_put(&fsg->ref, fsg_release); | 4276 | kref_put(&fsg->ref, fsg_release); |
4289 | } | 4277 | } |
4290 | module_exit(fsg_cleanup); | 4278 | module_exit(fsg_cleanup); |