search for: __get_user

...om victim which shares mm with the vhost kernel thread because it could see a zero page unexpectedly and theoretically make an incorrect decision visible outside of the killed task context. Make sure that each place which can read from userspace is annotated properly and it uses copy_from_user_mm, __get_user_mm resp. copy_from_iter_mm. Each will get the target mm as an argument and it performs a pessimistic check to rule out that the oom_reaper could possibly unmap the particular page. __oom_reap_task then just needs to mark the mm as unstable before it unmaps any page. This is a preparatory patch wit...

...om victim which shares mm with the vhost kernel thread because it could see a zero page unexpectedly and theoretically make an incorrect decision visible outside of the killed task context. Make sure that each place which can read from userspace is annotated properly and it uses copy_from_user_mm, __get_user_mm resp. copy_from_iter_mm. Each will get the target mm as an argument and it performs a pessimistic check to rule out that the oom_reaper could possibly unmap the particular page. __oom_reap_task then just needs to mark the mm as unstable before it unmaps any page. This is a preparatory patch wit...

...(status) flags and makes some > > decisions based on the read value. I do not understand the code so I > > couldn't evaluate whether that would lead to some real problems so I > > conservatively assumed it wouldn't handle that gracefully. > > Getting an error from __get_user and friends is handled gracefully. > Getting zero instead of a real value will cause userspace > memory corruption. OK, thanks for the confirmation! I will add this to the changelog. I assume that the memory corruption could "leak out" of the mm we just read from, right? I am askin...

...(status) flags and makes some > > decisions based on the read value. I do not understand the code so I > > couldn't evaluate whether that would lead to some real problems so I > > conservatively assumed it wouldn't handle that gracefully. > > Getting an error from __get_user and friends is handled gracefully. > Getting zero instead of a real value will cause userspace > memory corruption. OK, thanks for the confirmation! I will add this to the changelog. I assume that the memory corruption could "leak out" of the mm we just read from, right? I am askin...

...with the vhost kernel thread because it could see a zero > page unexpectedly and theoretically make an incorrect decision visible > outside of the killed task context. > > Make sure that each place which can read from userspace is annotated > properly and it uses copy_from_user_mm, __get_user_mm resp. > copy_from_iter_mm. Each will get the target mm as an argument and it > performs a pessimistic check to rule out that the oom_reaper could > possibly unmap the particular page. __oom_reap_task then just needs to > mark the mm as unstable before it unmaps any page. > > T...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...7 +1047,13 @@ int vhost_init_used(struct vhost_virtqueue *vq) if (r) return r; vq->signalled_used_valid = false; - return get_user(vq->last_used_idx, &vq->used->idx); + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) + return -EFAULT; + r = __get_user(last_used_idx, &vq->used->idx); + if (r) + return r; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); + return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 addr, u32 len, /* Each buffer in the v...

...truct vhost_virtqueue *vq) > if (r) > return r; > vq->signalled_used_valid = false; > - return get_user(vq->last_used_idx, &vq->used->idx); > + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) > + return -EFAULT; > + r = __get_user(last_used_idx, &vq->used->idx); > + if (r) > + return r; > + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); > + return 0; > } > EXPORT_SYMBOL_GPL(vhost_init_used); > > @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64...

...truct vhost_virtqueue *vq) > if (r) > return r; > vq->signalled_used_valid = false; > - return get_user(vq->last_used_idx, &vq->used->idx); > + if (!access_ok(VERIFY_READ, &vq->used->idx, sizeof vq->used->idx)) > + return -EFAULT; > + r = __get_user(last_used_idx, &vq->used->idx); > + if (r) > + return r; > + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); > + return 0; > } > EXPORT_SYMBOL_GPL(vhost_init_used); > > @@ -1087,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64...

...vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event) int vhost_init_used(struct vhost_virtqueue *vq) { - u16 last_used_idx; + __virtio16 last_used_idx; int r; if (!vq->private_data) return 0; @@ -1052,7 +1052,7 @@ int vhost_init_used(struct vhost_virtqueue *vq) r = __get_user(last_used_idx, &vq->used->idx); if (r) return r; - vq->last_used_idx = last_used_idx; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1094,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 a...

...vhost_update_avail_event(struct vhost_virtqueue *vq, u16 avail_event) int vhost_init_used(struct vhost_virtqueue *vq) { - u16 last_used_idx; + __virtio16 last_used_idx; int r; if (!vq->private_data) return 0; @@ -1052,7 +1052,7 @@ int vhost_init_used(struct vhost_virtqueue *vq) r = __get_user(last_used_idx, &vq->used->idx); if (r) return r; - vq->last_used_idx = last_used_idx; + vq->last_used_idx = vhost16_to_cpu(vq, last_used_idx); return 0; } EXPORT_SYMBOL_GPL(vhost_init_used); @@ -1094,16 +1094,16 @@ static int translate_desc(struct vhost_virtqueue *vq, u64 a...

...dd_used_and_signal_n(struct vhost_dev *dev, >> } >> EXPORT_SYMBOL_GPL(vhost_add_used_and_signal_n); >> >> +bool vhost_vq_more_avail(struct vhost_dev *dev, struct vhost_virtqueue *vq) >> +{ >> + __virtio16 avail_idx; >> + int r; >> + >> + r = __get_user(avail_idx, &vq->avail->idx); >> + if (r) { >> + vq_err(vq, "Failed to check avail idx at %p: %d\n", >> + &vq->avail->idx, r); >> + return false; > In patch 3 you are calling this under preempt disable, > so this actually can fail...