Skip to contentSkip to conversation

Anyone grok pf_batch_read() in xfs_repair?

DISCUSSION Eric Sandeen 1 participant 2 messages Aug 27, 2013 – Aug 29, 2013

lore
Share & tools
lore
#0 lore raw

There's a pretty clear buffer overflow in pf_batch_read() but I'm having a hard time wrapping my head around what it's supposed to be doing:

<snip>

                num = 0;
                if (which == PF_SECONDARY) {
                        bplist[0] = btree_find(args->io_queue, 0, &fsbno);
                        max_fsbno = MIN(fsbno + pf_max_fsbs,
                                                        args->last_bno_read);
                } else {
                        bplist[0] = btree_find(args->io_queue,
                                                args->last_bno_read, &fsbno);
                        max_fsbno = fsbno + pf_max_fsbs;
                }
                while (bplist[num] && num < MAX_BUFS && fsbno < max_fsbno) {
                        if (which != PF_META_ONLY ||
                            !B_IS_INODE(XFS_BUF_PRIORITY(bplist[num])))
                                num++;
                        bplist[num] = btree_lookup_next(args->io_queue, &fsbno);
                }

<snip>

That while loop is busted; we can increase num from (MAX_BUFS-1) to MAX_BUFS, and then index the bplist array with MAX_BUFS, which is 1 past its end - this corrupts memory. Derp!

But I'm just not grokking what this is supposed to be doing.

If we come in w/ (which == PF_META_ONLY) we'll immediately overwrite bplist[0] set in the if/else case above. It all seems weird. Anybody understand this offhand or should I just keep digging?

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

#1 lore raw
Quoted 3 lines · Eric Sandeen

There's a pretty clear buffer overflow in pf_batch_read() but I'm having a hard time wrapping my head around what it's supposed to be doing:

Head wrapped sufficiently now, I'll send a patch to fix it tomorrow.

-Eric

Quoted 29 lines

<snip>

               num = 0;
               if (which == PF_SECONDARY) {
                       bplist[0] = btree_find(args->io_queue, 0, &fsbno);
                       max_fsbno = MIN(fsbno + pf_max_fsbs,
                                                       args->last_bno_read);
               } else {
                       bplist[0] = btree_find(args->io_queue,
                                               args->last_bno_read, &fsbno);
                       max_fsbno = fsbno + pf_max_fsbs;
               }
               while (bplist[num] && num < MAX_BUFS && fsbno < max_fsbno) {
                       if (which != PF_META_ONLY ||
                           !B_IS_INODE(XFS_BUF_PRIORITY(bplist[num])))
                               num++;
                       bplist[num] = btree_lookup_next(args->io_queue, &fsbno);
               }

<snip>

That while loop is busted; we can increase num from (MAX_BUFS-1) to MAX_BUFS, and then index the bplist array with MAX_BUFS, which is 1 past its end - this corrupts memory. Derp!

But I'm just not grokking what this is supposed to be doing.

If we come in w/ (which == PF_META_ONLY) we'll immediately overwrite bplist[0] set in the if/else case above. It all seems weird. Anybody understand this offhand or should I just keep digging?

Quoted 7 lines

-Eric

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs

_______________________________________________
xfs mailing list
[email protected]
http://oss.sgi.com/mailman/listinfo/xfs