On Tue, Dec 13, 2016 at 09:09:25AM -0800, Linus Torvalds wrote:
> On Tue, Dec 13, 2016 at 9:05 AM, Al Viro <viro@xxxxxxxxxxxxxxxxxx> wrote:
> >
> > I really doubt it - there might be something subtle I'd missed, but AFAICS
> > it is vulnerable to the scenario above.
>
> Hmm. So maybe just add
>
> if (segment_eq(get_fs(), KERNEL_DS))
> return -EINVAL;
>
> to blk_rq_map_user_iov()?
To sg_write(), actually. And we need it both in old branches and in
the mainline - sg_new_write() problem applies in mainline as well, and
if nothing else, it allows to perform reads from arbitrary kernel
space address; I'm not sure how easy it is to see the value it has
fetched, but it certainly can do unpleasant things to memory-mapped
IO registers.
As the matter of fact, bsg_write() is no better - blk_fill_sgv4_hdr_rq()
is called before we get to blk_rq_map_user() where your test would make
it fail, and it does
if (copy_from_user(rq->cmd, (void __user *)(unsigned long)hdr->request,
hdr->request_len))
return -EFAULT;
with *hdr, including hrd->request, has come from the data fed to bsg_write().
So I think we should make both sg_write() and bsg_write() to fail as early
as possible when called with KERNEL_DS, mainline and all -stable branches.
--
To unsubscribe from this list: send the line "unsubscribe stable" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html