If a task read/write data in blocking mode, it will wait the completion in ffs_epfile_io(), if function unbind occurs, ffs_func_unbind() will kfree ffs ep, once the task wake up, it still dereference the ffs ep to obtain the request status. Fix it by moving the request status to io_data which is stack-safe.
Unified · Split
| @@ -122,8 +122,6 @@ struct ffs_ep { · 121 unmodified lines | |||
| 122 | struct usb_endpoint_descriptor *descs[3]; | 122 | struct usb_endpoint_descriptor *descs[3]; |
| 123 | 123 | ||
| 124 | u8 num; | 124 | u8 num; |
| 125 | |||
| 126 | int status; /* P: epfile->mutex */ | ||
| 127 | }; | 125 | }; |
| 128 | 126 | ||
| 129 | struct ffs_epfile { | 127 | struct ffs_epfile { |
| @@ -227,6 +225,9 @@ struct ffs_io_data { · 97 unmodified lines | |||
| 227 | bool use_sg; | 225 | bool use_sg; |
| 228 | 226 | ||
| 229 | struct ffs_data *ffs; | 227 | struct ffs_data *ffs; |
| 228 | |||
| 229 | int status; | ||
| 230 | struct completion done; | ||
| 230 | }; | 231 | }; |
| 231 | 232 | ||
| 232 | struct ffs_desc_helper { | 233 | struct ffs_desc_helper { |
| @@ -707,12 +708,15 @@ static const struct file_operations ffs_ep0_operations = { · 474 unmodified lines | |||
| 707 | 708 | ||
| 708 | static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) | 709 | static void ffs_epfile_io_complete(struct usb_ep *_ep, struct usb_request *req) |
| 709 | { | 710 | { |
| 711 | struct ffs_io_data *io_data = req->context; | ||
| 712 | |||
| 710 | ENTER(); | 713 | ENTER(); |
| 711 | if (req->context) { | 714 | if (req->status) |
| 712 | struct ffs_ep *ep = _ep->driver_data; | 715 | io_data->status = req->status; |
| 713 | ep->status = req->status ? req->status : req->actual; | 716 | else |
| 714 | complete(req->context); | 717 | io_data->status = req->actual; |
| 715 | } | 718 | |
| 719 | complete(&io_data->done); | ||
| 716 | } | 720 | } |
| 717 | 721 | ||
| 718 | static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) | 722 | static ssize_t ffs_copy_to_iter(void *data, int data_len, struct iov_iter *iter) |
| @@ -1050,7 +1054,6 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) · 331 unmodified lines | |||
| 1050 | WARN(1, "%s: data_len == -EINVAL\n", __func__); | 1054 | WARN(1, "%s: data_len == -EINVAL\n", __func__); |
| 1051 | ret = -EINVAL; | 1055 | ret = -EINVAL; |
| 1052 | } else if (!io_data->aio) { | 1056 | } else if (!io_data->aio) { |
| 1053 | DECLARE_COMPLETION_ONSTACK(done); | ||
| 1054 | bool interrupted = false; | 1057 | bool interrupted = false; |
| 1055 | 1058 | ||
| 1056 | req = ep->req; | 1059 | req = ep->req; |
| @@ -1066,7 +1069,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) · 9 unmodified lines | |||
| 1066 | 1069 | ||
| 1067 | io_data->buf = data; | 1070 | io_data->buf = data; |
| 1068 | 1071 | ||
| 1069 | req->context = &done; | 1072 | init_completion(&io_data->done); |
| 1073 | req->context = io_data; | ||
| 1070 | req->complete = ffs_epfile_io_complete; | 1074 | req->complete = ffs_epfile_io_complete; |
| 1071 | 1075 | ||
| 1072 | ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC); | 1076 | ret = usb_ep_queue(ep->ep, req, GFP_ATOMIC); |
| @@ -1075,7 +1079,7 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) · 2 unmodified lines | |||
| 1075 | 1079 | ||
| 1076 | spin_unlock_irq(&epfile->ffs->eps_lock); | 1080 | spin_unlock_irq(&epfile->ffs->eps_lock); |
| 1077 | 1081 | ||
| 1078 | if (wait_for_completion_interruptible(&done)) { | 1082 | if (wait_for_completion_interruptible(&io_data->done)) { |
| 1079 | /* | 1083 | /* |
| 1080 | * To avoid race condition with ffs_epfile_io_complete, | 1084 | * To avoid race condition with ffs_epfile_io_complete, |
| 1081 | * dequeue the request first then check | 1085 | * dequeue the request first then check |
| @@ -1083,17 +1087,17 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data) · 1 unmodified line | |||
| 1083 | * condition with req->complete callback. | 1087 | * condition with req->complete callback. |
| 1084 | */ | 1088 | */ |
| 1085 | usb_ep_dequeue(ep->ep, req); | 1089 | usb_ep_dequeue(ep->ep, req); |
| 1086 | wait_for_completion(&done); | 1090 | wait_for_completion(&io_data->done); |
| 1087 | interrupted = ep->status < 0; | 1091 | interrupted = io_data->status < 0; |
| 1088 | } | 1092 | } |
| 1089 | 1093 | ||
| 1090 | if (interrupted) | 1094 | if (interrupted) |
| 1091 | ret = -EINTR; | 1095 | ret = -EINTR; |
| 1092 | else if (io_data->read && ep->status > 0) | 1096 | else if (io_data->read && io_data->status > 0) |
| 1093 | ret = __ffs_epfile_read_data(epfile, data, ep->status, | 1097 | ret = __ffs_epfile_read_data(epfile, data, io_data->status, |
| 1094 | &io_data->data); | 1098 | &io_data->data); |
| 1095 | else | 1099 | else |
| 1096 | ret = ep->status; | 1100 | ret = io_data->status; |
| 1097 | goto error_mutex; | 1101 | goto error_mutex; |
| 1098 | } else if (!(req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC))) { | 1102 | } else if (!(req = usb_ep_alloc_request(ep->ep, GFP_ATOMIC))) { |
| 1099 | ret = -ENOMEM; | 1103 | ret = -ENOMEM; |