On 12/26/2012 09:16 PM, Nigel Tamplin wrote: > On 26/12/12 22:25, Alex Elder wrote: >> Nigel Tamplin reported getting a seg fault in xfsrestore when a path >> name was too long. >> >> Based on the surrounding code, I'm sure strerror(errno) was the >> intended final argument to this call. This bug has been there >> since the code was first committed. >> >> Signed-off-by: Alex Elder<elder@xxxxxxxxxxx> >> Reported-by: Nigel Tamplin<ntamplin@xxxxxxxxxxxxxxx> >> --- >> restore/content.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/restore/content.c b/restore/content.c >> index edd00ed..4e55a76 100644 >> --- a/restore/content.c >> +++ b/restore/content.c >> @@ -7799,7 +7799,8 @@ restore_spec( filehdr_t *fhdrp, rv_t *rvp, char >> *path ) >> "%s ino %llu %s: %s: discarding\n"), >> printstr, >> fhdrp->fh_stat.bs_ino, >> - path ); >> + path, >> + strerror( errno )); >> ( void )close( sockfd ); >> return BOOL_TRUE; >> } >> > > > Hi Alex, > > You say... >> Based on the surrounding code, I'm sure strerror(errno) was the > intended final argument to this call. > > I'm not so sure. > > The condition is comparing path length to max length, not detecting a > previously failed system call (like the surrounding code)- as such I > don't think the value of errno is relevant and the fix should be the > removal of one of the format specifiers instead of the addition of the > argument. Next time I'll actually read the code. Yes, I fully concur with your analysis. Do you want to submit a patch? Or do you want me to re-do mine? -Alex > Here's a contrived example: > > # cd /tmp > root@tulip:/tmp# mkdir mnt1 > root@tulip:/tmp# mkdir mnt2 > root@tulip:/tmp# dd if=/dev/zero of=block1 bs=4096 count=4096 > root@tulip:/tmp# dd if=/dev/zero of=block2 bs=4096 count=4096 > root@tulip:/tmp# mount -o loop /tmp/block1 /tmp/mnt1 > root@tulip:/tmp# mount -o loop /tmp/block2 /tmp/mnt2 > root@tulip:/tmp# socat UNIX-LISTEN:/tmp/mnt1/socket STDOUT & > root@tulip:/tmp# mv /tmp/mnt1/socket > /tmp/mnt1/11111111112222222222333333333344444444445555555555666666666677777777778888888888999999999900000000AAAAAAAAAA > > root@tulip:/tmp# xfsdump -J - /tmp/mnt1 | xfsrestore -J - /tmp/mnt2 > > xfsdump: using file dump (drive_simple) strategy > xfsdump: version 3.0.4 (dump format 3.0) - Running single-threaded > xfsrestore: using file dump (drive_simple) strategy > xfsdump: xfsrestore: version 3.0.4 (dump format 3.0) - Running > single-threaded > xfsrestore: searching media for dump > level 0 dump of acer:/tmp/mnt1 > xfsdump: dump date: Thu Dec 27 02:03:48 2012 > xfsdump: session id: e64a2ce0-514d-47eb-8464-b3d393b11005 > xfsdump: session label: "" > xfsdump: ino map phase 1: constructing initial dump list > xfsdump: ino map phase 2: skipping (no pruning necessary) > xfsdump: ino map phase 3: skipping (only one dump stream) > xfsdump: ino map construction complete > xfsdump: estimated dump size: 21120 bytes > xfsdump: creating dump session media file 0 (media 0, file 0) > xfsdump: dumping ino map > xfsdump: dumping directories > xfsdump: dumping non-directory files > xfsdump: ending media file > xfsrestore: examining media file 0 > xfsrestore: dump description: > xfsrestore: hostname: acer > xfsrestore: mount point: /tmp/mnt1 > xfsrestore: volume: /dev/loop0 > xfsrestore: session time: Thu Dec 27 02:03:48 2012 > xfsrestore: level: 0 > xfsrestore: session label: "" > xfsrestore: media label: "" > xfsrestore: file system id: 59c86d2c-63c0-4ed6-a4e3-c339120db582 > xfsrestore: session id: e64a2ce0-514d-47eb-8464-b3d393b11005 > xfsrestore: media id: 2c893d31-7f24-4c69-a639-4464ea253683 > xfsrestore: searching media for directory dump > xfsrestore: NOTE: attempt to reserve 4152 bytes for > /tmp/mnt2/xfsrestorehousekeepingdir/dirattr using XFS_IOC_RESVSP64 > failed: Operation not supported (95) > xfsrestore: NOTE: attempt to reserve 4116 bytes for > /tmp/mnt2/xfsrestorehousekeepingdir/namreg using XFS_IOC_RESVSP64 > failed: Operation not supported (95) > xfsrestore: reading directories > xfsrestore: 1 directories and 1 entries processed > xfsrestore: directory post-processing > xfsrestore: restoring non-directory files > xfsrestore: WARNING: pathname too long for bind of UNIX domain socket > ino 19332 > 11111111112222222222333333333344444444445555555555666666666677777777778888888888999999999900000000AAAAAAAAAA: > (null): discarding > xfsrestore: restore complete: 1 seconds elapsed > xfsrestore: Restore Status: SUCCESS > xfsdump: media file size 21400 bytes > xfsdump: dump size (non-dir files) : 0 bytes > xfsdump: dump complete: 1 seconds elapsed > xfsdump: Dump Status: SUCCESS > root@tulip:/tmp# > > > unpatched xfsrestore outputs > xfsrestore: WARNING: pathname too long for bind of UNIX domain socket > ino 19332 > 11111111112222222222333333333344444444445555555555666666666677777777778888888888999999999900000000AAAAAAAAAA: > (null): discarding > > note the "(null)". > (I fully expected this to segfault like it does on the dump file that > caused me to notice this bug in the first place, but this being an > example rather than 4 hours into an actual restore just before the > holidays, it gets away with displaying "null" - how I wish it had been > the other way around...) > > > with the patch as per your previous email xfsrestore outputs > xfsrestore: WARNING: pathname too long for bind of UNIX domain socket > ino 19332 > 11111111112222222222333333333344444444445555555555666666666677777777778888888888999999999900000000AAAAAAAAAA: > No such file or directory: discarding > > But I've a feeling the "No such file or directory:" message is unrelated > and just happens to be a left over value in errno. > > > whereas by removing the final specifier instead of adding the extra > argument then xfrestore outputs > xfsrestore: WARNING: pathname too long for bind of UNIX domain socket > ino 19332 > 11111111112222222222333333333344444444445555555555666666666677777777778888888888999999999900000000AAAAAAAAAA: > discarding > > Which I think is the correct message (with no chance of a left over > errno value confusing it) > > What do you think ? > Either way this is a minor issue now as either adding the argument or > removing the format specifier avoids the segfault which was fatal. > > Nigel > _______________________________________________ xfs mailing list xfs@xxxxxxxxxxx http://oss.sgi.com/mailman/listinfo/xfs
