From: Michael Wu <michael@allwinnertech.com>
To: balbi@kernel.org, gregkh@linuxfoundation.org, john@metanate.com,
axboe@kernel.dk, quic_pkondeti@quicinc.com,
wcheng@codeaurora.org, quic_ugoswami@quicinc.com,
andrew_gabbasov@mentor.com, plr.vincent@gmail.com
Cc: gustavoars@kernel.org, linux-usb@vger.kernel.org,
linux-kernel@vger.kernel.org,
allwinner-opensource-support@allwinnertech.com
Subject: [PATCH] usb: f_fs: Fix crash during gadget function switching
Date: Tue, 10 May 2022 16:01:05 +0800 [thread overview]
Message-ID: <20220510080105.126146-1-michael@allwinnertech.com> (raw)
On arm64 android12 and possibly other platforms, during the usb gadget
function switching procedure (e.g. from mtp to midi), a synchronization
issue could occur, which causes an use-after-free panic as shown below:
[ 119.787946][ T1] init: Control message: Processed ctl.start for 'adbd' from pid: 395 (system_server)
[ 119.790006][ T32] android_work: sent uevent USB_STATE=DISCONNECTED
[ 119.805457][ T2309] usercopy: Kernel memory exposure attempt detected from SLUB object 'kmalloc-512' (offset 0, size 1802201963)!
[ 119.819368][ T2309] ------------[ cut here ]------------
[ 119.825359][ T2309] kernel BUG at mm/usercopy.c:99!
[ 119.830868][ T2309] Internal error: Oops - BUG: 0 [#1] PREEMPT SMP
[ 119.837843][ T2309] Modules linked in: sunxi_usbc ohci_sunxi ehci_sunxi sunxi_hci mali_kbase(O) sunxi_rfkill
[ 119.848904][ T2309] CPU: 3 PID: 2309 Comm: MtpServer Tainted: G O 5.4.125+ #9
[ 119.858411][ T2309] Hardware name: sun50iw9 (DT)
[ 119.863615][ T2309] pstate: 60400005 (nZCv daif +PAN -UAO)
[ 119.869790][ T2309] pc : usercopy_abort+0x90/0x94
[ 119.875078][ T2309] lr : usercopy_abort+0x90/0x94
[ 119.880357][ T2309] sp : ffffffc019e13ad0
[ 119.884858][ T2309] x29: ffffffc019e13ae0 x28: 0000000000000000
[ 119.891601][ T2309] x27: ffffff8030e7c030 x26: 0000000000000000
[ 119.898347][ T2309] x25: 0000000000000001 x24: ffffff806e25bd30
[ 119.905092][ T2309] x23: 000000006b6b6b6b x22: ffffff807c804680
[ 119.911838][ T2309] x21: ffffff8041410200 x20: 0000000000000001
[ 119.918581][ T2309] x19: 000000006b6b6b6b x18: ffffffc019e05058
[ 119.925326][ T2309] x17: 0000000000000000 x16: 00000000000002e3
[ 119.932073][ T2309] x15: 0000000000000000 x14: 0000000000000010
[ 119.938820][ T2309] x13: ffffffc0103972c0 x12: 0000000000000001
[ 119.945565][ T2309] x11: 0000000000000000 x10: 0000000000000002
[ 119.952311][ T2309] x9 : 54fea1fa84857d00 x8 : 54fea1fa84857d00
[ 119.959056][ T2309] x7 : 0000000000000001 x6 : 0000000000000000
[ 119.965801][ T2309] x5 : ffffff807f3a9038 x4 : 0000000000000000
[ 119.972546][ T2309] x3 : 0000000000000000 x2 : ffffff807f3a8f40
[ 119.979293][ T2309] x1 : ffffffc0114e94de x0 : 000000000000006d
[ 119.986038][ T2309] Call trace:
[ 119.989569][ T2309] usercopy_abort+0x90/0x94
[ 119.994466][ T2309] __check_heap_object+0x16c/0x188
[ 120.000041][ T2309] __check_object_size+0x120/0x210
[ 120.005620][ T2309] ffs_epfile_io+0x574/0x6bc
[ 120.010612][ T2309] ffs_epfile_read_iter+0x10c/0x198
[ 120.016284][ T2309] __vfs_read+0x178/0x1e0
[ 120.020980][ T2309] vfs_read+0x1d0/0x284
[ 120.025481][ T2309] ksys_read+0x74/0xe0
[ 120.029884][ T2309] __arm64_sys_read+0x1c/0x28
[ 120.034977][ T2309] el0_svc_common+0xb8/0x1cc
[ 120.039967][ T2309] el0_svc_compat_handler+0x1c/0x28
[ 120.045649][ T2309] el0_svc_compat+0x8/0x24
[ 120.050453][ T2309] Code: f90003e4 aa0803e1 aa0903e4 97fa0fa9 (d4210000)
[ 120.058109][ T2309] ---[ end trace a246be823ca7d164 ]---
[ 120.064073][ T2309] Kernel panic - not syncing: Fatal exception
[ 120.070723][ T2309] SMP: stopping secondary CPUs
[ 120.076030][ T2309] Kernel Offset: disabled
[ 120.080757][ T2309] CPU features: 0x00010002,20002004
[ 120.086438][ T2309] Memory Limit: none
[ 120.090657][ T2309] ---[ end Kernel panic - not syncing: Fatal exception ]---
Two processes P1 and P2 engaged in the switching procedure:
- P1 (e.g. MtpServer) calls 'ffs_epfile_io' (alias 'C'),
- P2 (e.g. usb@1.0-service) calls 'ffs_epfile_io_complete' (alias 'A')
and the 'ffs_func_unbind' (alias 'B').
P1 P2
| |
| configfs_write_file|
| gadget_dev_desc_UDC_store|
| unregister_gadget|
| usb_gadget_unregister_driver|
| usb_gadget_remove_driver|
| |
| |________________________________________
| | |
| usb_gadget_disconnect| |
| sunxi_udc_pullup| |
| sunxi_udc_set_pullup| |
| configfs_composite_disconnect| |
| composite_disconnect| |
| reset_config| |
| ffs_func_disable| |
| ffs_func_set_alt| |
| ffs_func_eps_disable| |
| usb_ep_disable| |
| sunxi_udc_ep_disable| |
| sunxi_udc_nuke| configfs_composite_unbind|
| sunxi_udc_done| |
| | |
|ffs_epfile_read_iter | purge_configs_funcs|
| | |
| complete | |
|ffs_epfile_io [C] <------------ |ffs_epfile_io_complete [A] |
| + | |
| + |
| + |
| + [1] complete |
| +++++++++++++++++++++++++++++++++++++++++++++++> ffs_func_unbind[B]|
| |
On gadget switching, 'A' will raise a completion, which wakes up 'P1' to
resume 'C'. Then 'C' references 'ep->status' for branch jumping etc.
On the other path, 'B' keeps running and exec 'kfree(func->eps)', which
actually free 'ep->status' referenced by 'C'.
This leads to the panic: `usercopy: Kernel memory exposure attempt detected
from SLUB object 'kmalloc-512' (offset 0, size 1802201963)!`
To fix this issue, we add a completion[1] to ensure that 'C' completes
before 'B'.
Signed-off-by: Michael Wu <michael@allwinnertech.com>
---
drivers/usb/gadget/function/f_fs.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/drivers/usb/gadget/function/f_fs.c b/drivers/usb/gadget/function/f_fs.c
index 4585ee3a444a8..c3e918cd00170 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -124,6 +124,7 @@ struct ffs_ep {
u8 num;
int status; /* P: epfile->mutex */
+ void *context;
};
struct ffs_epfile {
@@ -246,6 +247,7 @@ ffs_sb_create_file(struct super_block *sb, const char *name, void *data,
DEFINE_MUTEX(ffs_lock);
EXPORT_SYMBOL_GPL(ffs_lock);
+DECLARE_COMPLETION(io_done);
static struct ffs_dev *_ffs_find_dev(const char *name);
static struct ffs_dev *_ffs_alloc_dev(void);
@@ -711,6 +713,8 @@ static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req)
if (req->context) {
struct ffs_ep *ep = _ep->driver_data;
ep->status = req->status ? req->status : req->actual;
+ if (ep->status == -ESHUTDOWN)
+ ep->context = &io_done;
complete(req->context);
}
}
@@ -1094,6 +1098,10 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
&io_data->data);
else
ret = ep->status;
+
+ if (ep->status == -ESHUTDOWN && ep->context)
+ complete(ep->context);
+
goto error_mutex;
} else if (!(req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC))) {
ret = -ENOMEM;
@@ -3607,6 +3615,12 @@ static void ffs_func_unbind(struct usb_configuration *c,
/* cleanup after autoconfig */
spin_lock_irqsave(&func->ffs->eps_lock, flags);
while (count--) {
+ if (ep->status == -ESHUTDOWN && ep->context) {
+ spin_unlock_irqrestore(&func->ffs->eps_lock, flags);
+ wait_for_completion(ep->context);
+ spin_lock_irqsave(&func->ffs->eps_lock, flags);
+ }
+
if (ep->ep && ep->req)
usb_ep_free_request(ep->ep, ep->req);
ep->req = NULL;
--
2.29.0
next reply other threads:[~2022-05-10 8:02 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-05-10 8:01 Michael Wu [this message]
2022-05-30 18:14 ` [PATCH] usb: f_fs: Fix crash during gadget function switching John Keeping
2022-06-02 10:36 ` Michael Wu
2022-06-02 13:05 ` John Keeping
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=20220510080105.126146-1-michael@allwinnertech.com \
--to=michael@allwinnertech.com \
--cc=allwinner-opensource-support@allwinnertech.com \
--cc=andrew_gabbasov@mentor.com \
--cc=axboe@kernel.dk \
--cc=balbi@kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=gustavoars@kernel.org \
--cc=john@metanate.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=plr.vincent@gmail.com \
--cc=quic_pkondeti@quicinc.com \
--cc=quic_ugoswami@quicinc.com \
--cc=wcheng@codeaurora.org \
/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.