linux-kernel - Re: [PATCH 2/4] mm: gup: add get_user_pages_locked and get_user_pages

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] [thread-next>] [day] [month] [year] [list]

Date:	Wed, 1 Oct 2014 10:06:27 -0700
From:	Andres Lagar-Cavilla <andreslc@...gle.com>
To:	Peter Feiner <pfeiner@...gle.com>
Cc:	Andrea Arcangeli <aarcange@...hat.com>, kvm@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-mm@...ck.org,
	Gleb Natapov <gleb@...nel.org>,
	Radim Krcmar <rkrcmar@...hat.com>,
	Paolo Bonzini <pbonzini@...hat.com>,
	Rik van Riel <riel@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Mel Gorman <mgorman@...e.de>,
	Andy Lutomirski <luto@...capital.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Sasha Levin <sasha.levin@...cle.com>,
	Jianyu Zhan <nasa4836@...il.com>,
	Paul Cassella <cassella@...y.com>,
	Hugh Dickins <hughd@...gle.com>,
	"\\Dr. David Alan Gilbert\\" <dgilbert@...hat.com>
Subject: Re: [PATCH 2/4] mm: gup: add get_user_pages_locked and get_user_pages_unlocked

On Wed, Oct 1, 2014 at 8:51 AM, Peter Feiner <pfeiner@...gle.com> wrote:
> On Wed, Oct 01, 2014 at 10:56:35AM +0200, Andrea Arcangeli wrote:
>> +static inline long __get_user_pages_locked(struct task_struct *tsk,
>> +                                        struct mm_struct *mm,
>> +                                        unsigned long start,
>> +                                        unsigned long nr_pages,
>> +                                        int write, int force,
>> +                                        struct page **pages,
>> +                                        struct vm_area_struct **vmas,
>> +                                        int *locked,
>> +                                        bool notify_drop)
>> +{
>> +     int flags = FOLL_TOUCH;
>> +     long ret, pages_done;
>> +     bool lock_dropped;
>> +
>> +     if (locked) {
>> +             /* if VM_FAULT_RETRY can be returned, vmas become invalid */
>> +             BUG_ON(vmas);
>> +             /* check caller initialized locked */
>> +             BUG_ON(*locked != 1);
>> +     }
>> +
>> +     if (pages)
>> +             flags |= FOLL_GET;
>> +     if (write)
>> +             flags |= FOLL_WRITE;
>> +     if (force)
>> +             flags |= FOLL_FORCE;
>> +
>> +     pages_done = 0;
>> +     lock_dropped = false;
>> +     for (;;) {
>> +             ret = __get_user_pages(tsk, mm, start, nr_pages, flags, pages,
>> +                                    vmas, locked);
>> +             if (!locked)
>> +                     /* VM_FAULT_RETRY couldn't trigger, bypass */
>> +                     return ret;
>> +
>> +             /* VM_FAULT_RETRY cannot return errors */
>> +             if (!*locked) {
>> +                     BUG_ON(ret < 0);
>> +                     BUG_ON(nr_pages == 1 && ret);
>
> If I understand correctly, this second BUG_ON is asserting that when
> __get_user_pages is asked for a single page and it is successfully gets the
> page, then it shouldn't have dropped the mmap_sem. If that's the case, then
> you could generalize this assertion to
>
>                         BUG_ON(nr_pages == ret);

Even more strict:
     BUG_ON(ret >= nr_pages);

Reviewed-by: Andres Lagar-Cavilla <andreslc@...gle.com>

>
> Otherwise, looks good!
>
> Peter



-- 
Andres Lagar-Cavilla | Google Kernel Team | andreslc@...gle.com
--
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/