linux-kernel - Re: [PATCH] aoe: adjust ref of head for compound page tails

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]

Message-Id: <20130807163507.954590c479a07886a8ae6cb1@linux-foundation.org>
Date:	Wed, 7 Aug 2013 16:35:07 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Ed Cashin <ecashin@...aid.com>
Cc:	<linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] aoe: adjust ref of head for compound page tails

On Wed, 7 Aug 2013 19:27:40 -0400 Ed Cashin <ecashin@...aid.com> wrote:

> On Aug 7, 2013, at 5:26 PM, Andrew Morton wrote:
> 
> > On Wed, 7 Aug 2013 17:21:32 -0400 Ed Cashin <ecashin@...aid.com> wrote:
> ...
> >> Sorry.  I tried to cover that but maybe should have separated it more clearly from the running text.
> >> 
> >> The patch changes the current behavior encountered with direct I/O and aoe:
> >> 
> >>  aoe can BUG when direct I/O used
> >> 
> >> ... to a new behavior:
> >> 
> >>  aoe does not BUG when direct I/O is used
> >> 
> >> And the user will never know that instead aoe is manipulating the _count of the head page, even though that's what's happening with this patch applied.
> >> 
> >> (The user-visible effects that motivated the older patch that added the BUG to aoe when page tails are used for block I/O was a VM_BUG_ON that occurred when the aoe driver manipulated the _count of a compound page tail.)
> > 
> > OK, I added "Fix a BUG which can trigger when direct-IO is used with
> > AOE".  Which kernel versions are affected?
> 
> 
> That BUG was added to the aoe driver in a commit that first appears in v3.7.  It persists until this patch adds the code that handles the situation that the BUG was intended to detect.
> 
> [ecashin@...ino linux]$ git describe --contains --match 'v*' 69cf2d85de773d998798e47e3335b85e5645d157
> v3.7-rc1~110^2~22
> [ecashin@...ino linux]$ 
> 

ah, is it this?

+bio_pageinc(struct bio *bio)
+{
+       struct bio_vec *bv;
+       struct page *page;
+       int i;
+
+       bio_for_each_segment(bv, bio, i) {
+               page = bv->bv_page;
+               /* Non-zero page count for non-head members of
+                * compound pages is no longer allowed by the kernel,
+                * but this has never been seen here.
+                */
+               if (unlikely(PageCompound(page)))
+                       if (compound_trans_head(page) != page) {
+                               pr_crit("page tail used for block I/O\n");
+                               BUG();
+                       }

But get_page/put_page against a tail page of a compound page should
Just Work.  The core MM will hunt down the head page and manipulate its
refcount.  

Perhaps the problem here is that AOE is diving under the covers and is
using low-level page refcount alterations:

+               atomic_inc(&page->_count);

Why does AOE do this?  It would be better if it were to use the
official published MM interfaces.  If those interfaces need
alteration/augmentation then we can do that.



--
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/