linux-kernel - Re: [PATCH] mm: get_user_pages() stores ERR

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <c384c5ea0908040925u4763f02fy5baaf418f0ad8aa5@mail.gmail.com>
Date:	Tue, 4 Aug 2009 18:25:17 +0200
From:	Leon Woestenberg <leon.woestenberg@...il.com>
To:	Hugh Dickins <hugh.dickins@...cali.co.uk>
Cc:	Brice Goglin <Brice.Goglin@...ia.fr>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] mm: get_user_pages() stores ERR_PTR() in pages[i] on 
	failure

Hello,

thanks for looking along on this one.

On Tue, Aug 4, 2009 at 2:00 PM, Hugh Dickins<hugh.dickins@...cali.co.uk> wrote:
> On Tue, 4 Aug 2009, Brice Goglin wrote:
>>
>> >> I wonder if we should change get_user_pages to store ERR_PTR(ret)
>> >> in page[i] when it fails to get page #i.
>> >>
>> > Yes, I would see that as an improvement in finding out why rc <
>> > nr_pages, in case rc > 0.
>> >
>> > Also I think it does not break existing users.
>> >
> ... some callers of get_user_pages() might expect it not to touch
> entries in the page array beyond the fail point: for example, they
> might start off with a zeroed array, then when they come to put_page
> on the array afterwards, would skip any NULL entries found - without
> having to have noted the returned number of pages.  I don't think
> that way of working is supported by any guarantee in documentation,
> but it's not unreasonable to expect it to behave in that way.
>
Thanks for checking this -- obviously me thinking it would not break
existing users is false.

> Of course we _could_ fix individual callers to work with it, but
> for me they sound a warning that it's too late to change now.
>
The alternative seems to be to describe a best practise of calling
get_user_pages() on the failing page to find out why it failed.
(and hoping it does not return rc > 0 on the second call.)

A better function API would be to return the number of succesfully
gotten pages in a pointer
rc = get_user_pages2(, nr_pages ,..., &nr_pages_gotten);
where rc ==0 iff (nr_pages == nr_pages_gotten).


BTW, in my case I'm hitting the (ret & VM_FAULT_SIGBUS) case. I
suspect I have a multithreading concurrency race, as I have DMA in
both directions to a ring buffer set of files. Seems one over runs the
other, thus the file gets mapped in with different protection, as well
as conflicting DMA directions.

-EBADUSER.
-- 
Leon
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/