Skip to content

Commit f788baa

Browse files
torvaldstorvalds
committed
Merge branch 'gadget' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs
Pull gadgetfs fixes from Al Viro: "Assorted fixes around AIO on gadgetfs: leaks, use-after-free, troubles caused by ->f_op flipping" * 'gadget' of git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs: gadgetfs: really get rid of switching ->f_op gadgetfs: get rid of flipping ->f_op in ep_config() gadget: switch ep_io_operations to ->read_iter/->write_iter gadgetfs: use-after-free in ->aio_read() gadget/function/f_fs.c: switch to ->{read,write}_iter() gadget/function/f_fs.c: use put iov_iter into io_data gadget/function/f_fs.c: close leaks move iov_iter.c from mm/ to lib/ new helper: dup_iter()
2 parents c202baf + 96b62a5 commit f788baa

File tree

6 files changed

+291
-400
lines changed

6 files changed

+291
-400
lines changed

‎drivers/usb/gadget/function/f_fs.c

Lines changed: 79 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -144,10 +144,9 @@ struct ffs_io_data {
144144
bool read;
145145

146146
struct kiocb *kiocb;
147-
const struct iovec *iovec;
148-
unsigned long nr_segs;
149-
char __user *buf;
150-
size_t len;
147+
struct iov_iter data;
148+
const void *to_free;
149+
char *buf;
151150

152151
struct mm_struct *mm;
153152
struct work_struct work;
@@ -649,29 +648,10 @@ static void ffs_user_copy_worker(struct work_struct *work)
649648
io_data->req->actual;
650649

651650
if (io_data->read && ret > 0) {
652-
int i;
653-
size_t pos = 0;
654-
655-
/*
656-
* Since req->length may be bigger than io_data->len (after
657-
* being rounded up to maxpacketsize), we may end up with more
658-
* data then user space has space for.
659-
*/
660-
ret = min_t(int, ret, io_data->len);
661-
662651
use_mm(io_data->mm);
663-
for (i = 0; i < io_data->nr_segs; i++) {
664-
size_t len = min_t(size_t, ret - pos,
665-
io_data->iovec[i].iov_len);
666-
if (!len)
667-
break;
668-
if (unlikely(copy_to_user(io_data->iovec[i].iov_base,
669-
&io_data->buf[pos], len))) {
670-
ret = -EFAULT;
671-
break;
672-
}
673-
pos += len;
674-
}
652+
ret = copy_to_iter(io_data->buf, ret, &io_data->data);
653+
if (iov_iter_count(&io_data->data))
654+
ret = -EFAULT;
675655
unuse_mm(io_data->mm);
676656
}
677657

@@ -684,7 +664,7 @@ static void ffs_user_copy_worker(struct work_struct *work)
684664

685665
io_data->kiocb->private = NULL;
686666
if (io_data->read)
687-
kfree(io_data->iovec);
667+
kfree(io_data->to_free);
688668
kfree(io_data->buf);
689669
kfree(io_data);
690670
}
@@ -743,41 +723,29 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
743723
* before the waiting completes, so do not assign to 'gadget' earlier
744724
*/
745725
struct usb_gadget *gadget = epfile->ffs->gadget;
726+
size_t copied;
746727

747728
spin_lock_irq(&epfile->ffs->eps_lock);
748729
/* In the meantime, endpoint got disabled or changed. */
749730
if (epfile->ep != ep) {
750731
spin_unlock_irq(&epfile->ffs->eps_lock);
751732
return -ESHUTDOWN;
752733
}
734+
data_len = iov_iter_count(&io_data->data);
753735
/*
754736
* Controller may require buffer size to be aligned to
755737
* maxpacketsize of an out endpoint.
756738
*/
757-
data_len = io_data->read ?
758-
usb_ep_align_maybe(gadget, ep->ep, io_data->len) :
759-
io_data->len;
739+
if (io_data->read)
740+
data_len = usb_ep_align_maybe(gadget, ep->ep, data_len);
760741
spin_unlock_irq(&epfile->ffs->eps_lock);
761742

762743
data = kmalloc(data_len, GFP_KERNEL);
763744
if (unlikely(!data))
764745
return -ENOMEM;
765-
if (io_data->aio && !io_data->read) {
766-
int i;
767-
size_t pos = 0;
768-
for (i = 0; i < io_data->nr_segs; i++) {
769-
if (unlikely(copy_from_user(&data[pos],
770-
io_data->iovec[i].iov_base,
771-
io_data->iovec[i].iov_len))) {
772-
ret = -EFAULT;
773-
goto error;
774-
}
775-
pos += io_data->iovec[i].iov_len;
776-
}
777-
} else {
778-
if (!io_data->read &&
779-
unlikely(__copy_from_user(data, io_data->buf,
780-
io_data->len))) {
746+
if (!io_data->read) {
747+
copied = copy_from_iter(data, data_len, &io_data->data);
748+
if (copied != data_len) {
781749
ret = -EFAULT;
782750
goto error;
783751
}
@@ -876,10 +844,8 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
876844
*/
877845
ret = ep->status;
878846
if (io_data->read && ret > 0) {
879-
ret = min_t(size_t, ret, io_data->len);
880-
881-
if (unlikely(copy_to_user(io_data->buf,
882-
data, ret)))
847+
ret = copy_to_iter(data, ret, &io_data->data);
848+
if (unlikely(iov_iter_count(&io_data->data)))
883849
ret = -EFAULT;
884850
}
885851
}
@@ -898,37 +864,6 @@ static ssize_t ffs_epfile_io(struct file *file, struct ffs_io_data *io_data)
898864
return ret;
899865
}
900866

901-
static ssize_t
902-
ffs_epfile_write(struct file *file, const char __user *buf, size_t len,
903-
loff_t *ptr)
904-
{
905-
struct ffs_io_data io_data;
906-
907-
ENTER();
908-
909-
io_data.aio = false;
910-
io_data.read = false;
911-
io_data.buf = (char * __user)buf;
912-
io_data.len = len;
913-
914-
return ffs_epfile_io(file, &io_data);
915-
}
916-
917-
static ssize_t
918-
ffs_epfile_read(struct file *file, char __user *buf, size_t len, loff_t *ptr)
919-
{
920-
struct ffs_io_data io_data;
921-
922-
ENTER();
923-
924-
io_data.aio = false;
925-
io_data.read = true;
926-
io_data.buf = buf;
927-
io_data.len = len;
928-
929-
return ffs_epfile_io(file, &io_data);
930-
}
931-
932867
static int
933868
ffs_epfile_open(struct inode *inode, struct file *file)
934869
{
@@ -965,67 +900,86 @@ static int ffs_aio_cancel(struct kiocb *kiocb)
965900
return value;
966901
}
967902

968-
static ssize_t ffs_epfile_aio_write(struct kiocb *kiocb,
969-
const struct iovec *iovec,
970-
unsigned long nr_segs, loff_t loff)
903+
static ssize_t ffs_epfile_write_iter(struct kiocb *kiocb, struct iov_iter *from)
971904
{
972-
struct ffs_io_data *io_data;
905+
struct ffs_io_data io_data, *p = &io_data;
906+
ssize_t res;
973907

974908
ENTER();
975909

976-
io_data = kmalloc(sizeof(*io_data), GFP_KERNEL);
977-
if (unlikely(!io_data))
978-
return -ENOMEM;
910+
if (!is_sync_kiocb(kiocb)) {
911+
p = kmalloc(sizeof(io_data), GFP_KERNEL);
912+
if (unlikely(!p))
913+
return -ENOMEM;
914+
p->aio = true;
915+
} else {
916+
p->aio = false;
917+
}
979918

980-
io_data->aio = true;
981-
io_data->read = false;
982-
io_data->kiocb = kiocb;
983-
io_data->iovec = iovec;
984-
io_data->nr_segs = nr_segs;
985-
io_data->len = kiocb->ki_nbytes;
986-
io_data->mm = current->mm;
919+
p->read = false;
920+
p->kiocb = kiocb;
921+
p->data = *from;
922+
p->mm = current->mm;
987923

988-
kiocb->private = io_data;
924+
kiocb->private = p;
989925

990926
kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
991927

992-
return ffs_epfile_io(kiocb->ki_filp, io_data);
928+
res = ffs_epfile_io(kiocb->ki_filp, p);
929+
if (res == -EIOCBQUEUED)
930+
return res;
931+
if (p->aio)
932+
kfree(p);
933+
else
934+
*from = p->data;
935+
return res;
993936
}
994937

995-
static ssize_t ffs_epfile_aio_read(struct kiocb *kiocb,
996-
const struct iovec *iovec,
997-
unsigned long nr_segs, loff_t loff)
938+
static ssize_t ffs_epfile_read_iter(struct kiocb *kiocb, struct iov_iter *to)
998939
{
999-
struct ffs_io_data *io_data;
1000-
struct iovec *iovec_copy;
940+
struct ffs_io_data io_data, *p = &io_data;
941+
ssize_t res;
1001942

1002943
ENTER();
1003944

1004-
iovec_copy = kmalloc_array(nr_segs, sizeof(*iovec_copy), GFP_KERNEL);
1005-
if (unlikely(!iovec_copy))
1006-
return -ENOMEM;
1007-
1008-
memcpy(iovec_copy, iovec, sizeof(struct iovec)*nr_segs);
1009-
1010-
io_data = kmalloc(sizeof(*io_data), GFP_KERNEL);
1011-
if (unlikely(!io_data)) {
1012-
kfree(iovec_copy);
1013-
return -ENOMEM;
945+
if (!is_sync_kiocb(kiocb)) {
946+
p = kmalloc(sizeof(io_data), GFP_KERNEL);
947+
if (unlikely(!p))
948+
return -ENOMEM;
949+
p->aio = true;
950+
} else {
951+
p->aio = false;
1014952
}
1015953

1016-
io_data->aio = true;
1017-
io_data->read = true;
1018-
io_data->kiocb = kiocb;
1019-
io_data->iovec = iovec_copy;
1020-
io_data->nr_segs = nr_segs;
1021-
io_data->len = kiocb->ki_nbytes;
1022-
io_data->mm = current->mm;
954+
p->read = true;
955+
p->kiocb = kiocb;
956+
if (p->aio) {
957+
p->to_free = dup_iter(&p->data, to, GFP_KERNEL);
958+
if (!p->to_free) {
959+
kfree(p);
960+
return -ENOMEM;
961+
}
962+
} else {
963+
p->data = *to;
964+
p->to_free = NULL;
965+
}
966+
p->mm = current->mm;
1023967

1024-
kiocb->private = io_data;
968+
kiocb->private = p;
1025969

1026970
kiocb_set_cancel_fn(kiocb, ffs_aio_cancel);
1027971

1028-
return ffs_epfile_io(kiocb->ki_filp, io_data);
972+
res = ffs_epfile_io(kiocb->ki_filp, p);
973+
if (res == -EIOCBQUEUED)
974+
return res;
975+
976+
if (p->aio) {
977+
kfree(p->to_free);
978+
kfree(p);
979+
} else {
980+
*to = p->data;
981+
}
982+
return res;
1029983
}
1030984

1031985
static int
@@ -1105,10 +1059,10 @@ static const struct file_operations ffs_epfile_operations = {
11051059
.llseek = no_llseek,
11061060

11071061
.open = ffs_epfile_open,
1108-
.write = ffs_epfile_write,
1109-
.read = ffs_epfile_read,
1110-
.aio_write = ffs_epfile_aio_write,
1111-
.aio_read = ffs_epfile_aio_read,
1062+
.write = new_sync_write,
1063+
.read = new_sync_read,
1064+
.write_iter = ffs_epfile_write_iter,
1065+
.read_iter = ffs_epfile_read_iter,
11121066
.release = ffs_epfile_release,
11131067
.unlocked_ioctl = ffs_epfile_ioctl,
11141068
};

0 commit comments

Comments
 (0)