[patch] xfsprogs: repair pagefault due to missed out sanity NULL check

[patch] xfsprogs: repair pagefault due to missed out sanity NULL check

[Ajeet Yadav]

[-- Attachment #1.1: Type: text/plain, Size: 1012 bytes --]

libxfs_putbuf() is called with bp = NULL, resulting in pagefault in
libpthread.

Function da_read_buf() allocate array of xfs_buf_t *

   * xfs_buf_t       **bplist;*

*    bplist = calloc(nex, sizeof(*bplist));*

Read and fill it using

*for (i = 0; i < nex; i++) {
    bplist[i] = libxfs_readbuf()*

*    if (!bplist[i]){
        goto failed;
    }  *

*}*

*failed:
        for (i = 0; i < nex; i++)
                libxfs_putbuf(bplist[i]);*

Now assume nex = 10,

1. Will create bplist for 10 array elements.

3. Reading from disk 0,1, 2, 3

4. When reading from disk 4, USB is removed

5. libxfs_readbuf() will at fail, pblist[4] = NULL, goto failed.

6. Since only 4 buffers were read successfully, so only 4 are in lock state.

7.  Error handling will unlock buffer from 1-10

8. Buffer 0-3 were read successfully, hence will have valid pdlist[i]

9. Access pblist[4] == NULL, therefore unlocking will set bp == NULL in
libxfs_putbuf(bp);
10. Page fault in libpthread


Solution patch attached with mail

[-- Attachment #1.2: Type: text/html, Size: 3608 bytes --]

[-- Attachment #2: xfs_repair_da_read_buf_failed_unlock_fix.patch --]
[-- Type: application/octet-stream, Size: 581 bytes --]

diff -Nurp xfsprogs-3.0.5/repair/dir2.c xfsprogs-3.0.5-dirty/repair/dir2.c
--- xfsprogs-3.0.5/repair/dir2.c	2010-07-16 13:07:09.000000000 +0900
+++ xfsprogs-3.0.5-dirty/repair/dir2.c	2011-01-28 18:49:21.000000000 +0900
@@ -110,9 +110,10 @@ da_read_buf(
 		bplist[i] = libxfs_readbuf(mp->m_dev,
 				XFS_FSB_TO_DADDR(mp, bmp[i].startblock),
 				XFS_FSB_TO_BB(mp, bmp[i].blockcount), 0);
-		if (!bplist[i])
+		if (!bplist[i]){
+			nex = i;
 			goto failed;
-
+		}
 		pftrace("readbuf %p (%llu, %d)", bplist[i],
 			(long long)XFS_BUF_ADDR(bplist[i]),
 			XFS_BUF_COUNT(bplist[i]));

[-- Attachment #3: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [patch] xfsprogs: repair pagefault due to missed out sanity NULL check

[Ajeet Yadav]

[-- Attachment #1.1: Type: text/plain, Size: 1786 bytes --]

I did not receive any response / review comment on solution patch I sent.

diff -Nurp xfsprogs/repair/dir2.c xfsprogs-dirty/repair/dir2.c

--- xfsprogs/repair/dir2.c 2010-07-16 13:07:09.000000000 +0900

+++ xfsprogs-dirty/repair/dir2.c 2011-01-28 18:49:21.000000000 +0900

@@ -110,9 +110,10 @@ da_read_buf(

bplist[i] = libxfs_readbuf(mp->m_dev,

XFS_FSB_TO_DADDR(mp, bmp[i].startblock),

XFS_FSB_TO_BB(mp, bmp[i].blockcount), 0);

- if (!bplist[i])

+ if (!bplist[i]){

+ nex = i;

goto failed;

-

+ }

pftrace("readbuf %p (%llu, %d)", bplist[i],

(long long)XFS_BUF_ADDR(bplist[i]),

XFS_BUF_COUNT(bplist[i]));
On Fri, Jan 28, 2011 at 8:13 PM, Ajeet Yadav <ajeet.yadav.77@gmail.com>wrote:

> libxfs_putbuf() is called with bp = NULL, resulting in pagefault in
> libpthread.
>
> Function da_read_buf() allocate array of xfs_buf_t *
>
>    * xfs_buf_t       **bplist;*
>
> *    bplist = calloc(nex, sizeof(*bplist));*
>
> Read and fill it using
>
> *for (i = 0; i < nex; i++) {
>     bplist[i] = libxfs_readbuf()*
>
> *    if (!bplist[i]){
>         goto failed;
>     }  *
>
> *}*
>
> *failed:
>         for (i = 0; i < nex; i++)
>                 libxfs_putbuf(bplist[i]);*
>
> Now assume nex = 10,
>
> 1. Will create bplist for 10 array elements.
>
> 3. Reading from disk 0,1, 2, 3
>
> 4. When reading from disk 4, USB is removed
>
> 5. libxfs_readbuf() will at fail, pblist[4] = NULL, goto failed.
>
> 6. Since only 4 buffers were read successfully, so only 4 are in lock
> state.
>
> 7.  Error handling will unlock buffer from 1-10
>
> 8. Buffer 0-3 were read successfully, hence will have valid pdlist[i]
>
> 9. Access pblist[4] == NULL, therefore unlocking will set bp == NULL in
> libxfs_putbuf(bp);
> 10. Page fault in libpthread
>
>
> Solution patch attached with mail
>
>
>

[-- Attachment #1.2: Type: text/html, Size: 4722 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [patch] xfsprogs: repair pagefault due to missed out sanity NULL check

[Dave Chinner]

On Fri, Jan 28, 2011 at 08:13:04PM +0900, Ajeet Yadav wrote:
> libxfs_putbuf() is called with bp = NULL, resulting in pagefault in
> libpthread.
> 
> Function da_read_buf() allocate array of xfs_buf_t *
> 
>    * xfs_buf_t       **bplist;*
> 
> *    bplist = calloc(nex, sizeof(*bplist));*
> 
> Read and fill it using
> 
> *for (i = 0; i < nex; i++) {
>     bplist[i] = libxfs_readbuf()*
> 
> *    if (!bplist[i]){
>         goto failed;
>     }  *
> 
> *}*
> 
> *failed:
>         for (i = 0; i < nex; i++)
>                 libxfs_putbuf(bplist[i]);*
> 
> Now assume nex = 10,
> 
> 1. Will create bplist for 10 array elements.
> 
> 3. Reading from disk 0,1, 2, 3
> 
> 4. When reading from disk 4, USB is removed
> 
> 5. libxfs_readbuf() will at fail, pblist[4] = NULL, goto failed.
> 
> 6. Since only 4 buffers were read successfully, so only 4 are in lock state.
> 
> 7.  Error handling will unlock buffer from 1-10
> 
> 8. Buffer 0-3 were read successfully, hence will have valid pdlist[i]
> 
> 9. Access pblist[4] == NULL, therefore unlocking will set bp == NULL in
> libxfs_putbuf(bp);
> 10. Page fault in libpthread
> 
> 
> Solution patch attached with mail

Can you please include the patches in-line in your email rather than
as base64 encoded attachments? Even though it is for xfsprogs, we
ask that the same process is followed as per kernel patches. That
includes addіng Signed-off-by tags to the patches...

See Documentation/SubmittingPatches:

| 7) No MIME, no links, no compression, no attachments.  Just plain text.
| 
| Linus and other kernel developers need to be able to read and comment
| on the changes you are submitting.  It is important for a kernel
| developer to be able to "quote" your changes, using standard e-mail
| tools, so that they may comment on specific portions of your code.
| 
| For this reason, all patches should be submitting e-mail "inline".
| WARNING:  Be wary of your editor's word-wrap corrupting your patch,
| if you choose to cut-n-paste your patch.
| 
| Do not attach the patch as a MIME attachment, compressed or not.
| Many popular e-mail applications will not always transmit a MIME
| attachment as plain text, making it impossible to comment on your
| code.  A MIME attachment also takes Linus a bit more time to process,
| decreasing the likelihood of your MIME-attached change being accepted.
| 
| Exception:  If your mailer is mangling patches then someone may ask
| you to re-send them using MIME.
| 
| See Documentation/email-clients.txt for hints about configuring
| your e-mail client so that it sends your patches untouched.

And as it suggests, read Documentation/email-clients.txt on how to
do this with various mail clients.

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [patch] xfsprogs: repair pagefault due to missed out sanity NULL check

[Ajeet Yadav]

[-- Attachment #1.1: Type: text/plain, Size: 1325 bytes --]

xfsprogs: repair pagefaults due to unhandled NULL check in da_read_buf()

xfs_repair does not correctly handle bplist[i] for error situations in
function da_read_buf(). If libxfs_readbuf() fails then bplist[i] = NULL,
but error handing code calls libxfs_putbuf(bdlist[i]) for all indexes of i
without first checking whether its NULL. This result in pagefault in
libpthread library during pthread_mutex_unlock().
This problem is identified when we remove the storage while xfs_repair
is running on it.

Signed-off-by: Ajeet Yadav <ajeet.yadav.77@gmail.com>

diff -Nurp xfsprogs/repair/dir2.c xfsprogs-dirty/repair/dir2.c
--- xfsprogs/repair/dir2.c      2010-07-16 13:07:09.000000000 +0900
+++ xfsprogs-dirty/repair/dir2.c        2011-01-28 18:49:21.000000000 +0900
@@ -110,9 +110,10 @@ da_read_buf(
                bplist[i] = libxfs_readbuf(mp->m_dev,
                                XFS_FSB_TO_DADDR(mp, bmp[i].startblock),
                                XFS_FSB_TO_BB(mp, bmp[i].blockcount), 0);
-               if (!bplist[i])
+               if (!bplist[i]){
+                       nex = i;
                        goto failed;
-
+               }
                pftrace("readbuf %p (%llu, %d)", bplist[i],
                        (long long)XFS_BUF_ADDR(bplist[i]),
                        XFS_BUF_COUNT(bplist[i]));

[-- Attachment #1.2: Type: text/html, Size: 1496 bytes --]

[-- Attachment #2: Type: text/plain, Size: 121 bytes --]

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs

Re: [patch] xfsprogs: repair pagefault due to missed out sanity NULL check

[Christoph Hellwig]

Thanks, applied.

_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs