Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 28 Jun 2009 22:14:16 +0200
From:      Thomas Backman <serenity@exscape.org>
To:        FreeBSD current <freebsd-current@freebsd.org>
Cc:        freebsd-fs@freebsd.org
Subject:   Re: zfs send -R segfault, anyone else?
Message-ID:  <09277772-9C54-4AE6-A147-CB6A4ED38C48@exscape.org>
In-Reply-To: <B7DA57FE-0109-44D2-A970-E7BE7F3C24C5@exscape.org>
References:  <08D1E6DF-89D3-4887-9234-C3DB9164D794@exscape.org>	<20090514133017.362075dhcdy7o2bs@webmail.leidinger.net> <7CD27FF0-CBFA-48B7-9E18-763D8C3ED9B8@exscape.org> <4A0C9B0C.4050403@jrv.org> <B7DA57FE-0109-44D2-A970-E7BE7F3C24C5@exscape.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On May 15, 2009, at 11:30 AM, Thomas Backman wrote:
>
> On May 15, 2009, at 12:28 AM, James R. Van Artsdalen wrote:
>
>> Thomas Backman wrote:
>>> [root@chaos ~]# zfs send -R -I $OLD tank@$NOW > diff-snap
>>> [root@chaos ~]# cat diff-snap | zfs recv -Fvd slave
>>> Segmentation fault: 11 (core dumped)
>>>
>>> Same kinda backtrace, but what's up with strcmp()?
>>> I suppose the issue stems from libzfs, and is not within libc:
>>
>> Different problem  The SIGSEGV is happening in strcmp because it is
>> called with strcmp(0,0)
>> and tries to dereference address -4 (probably another bug itself).
>>
>> This hack gets around the issue but someone familiar with this  
>> needs to
>> decide the correct action.
>>
>> The first change is actually unrelated (a sorry attempt at fixing the
>> previous zfs send bug).
>>
>> The last change may be unnecessary as that case may never happen  
>> unless
>> the pool can be renamed?
>>
>> [... patch ...]
>
> Thanks! This list is pretty impressive. :)
> I can't validate how correct the fix is, considering my lacking  
> knowledge in C (I know the basics, but kernel/related programming?  
> no way!), but I CAN say that it appears to work just fine!
>
> Regards,
> Thomas
>
Any news on this? The bug's been around for a long time, and a fix has  
been around for at least 1.5 months now, and AFAIK the bug still lives.
The patch, again (I can't vouch for its correctness, but I can  
certainly say that it works just fine *for me*) follows.

Regards,
Thomas

Index: cddl/contrib/opensolaris/lib/libzfs/common/libzfs_sendrecv.c
===================================================================
--- cddl/contrib/opensolaris/lib/libzfs/common/ 
libzfs_sendrecv.c        (revision 194851)
+++ cddl/contrib/opensolaris/lib/libzfs/common/ 
libzfs_sendrecv.c        (working copy)
@@ -239,6 +239,8 @@
                 char *propname = nvpair_name(elem);
                 zfs_prop_t prop = zfs_name_to_prop(propname);
                 nvlist_t *propnv;
+               if (prop == ZPROP_INVAL)
+                   continue;

                 if (!zfs_prop_user(propname) &&  
zfs_prop_readonly(prop))
                         continue;
@@ -1126,7 +1128,7 @@
                 uint64_t originguid = 0;
                 uint64_t stream_originguid = 0;
                 uint64_t parent_fromsnap_guid,  
stream_parent_fromsnap_guid;
-               char *fsname, *stream_fsname;
+               char *fsname, *stream_fsname, *p1, *p2;

                 nextfselem = nvlist_next_nvpair(local_nv, fselem);

@@ -1295,10 +1297,13 @@
                     "parentfromsnap", &stream_parent_fromsnap_guid));

                 /* check for rename */
+               p1 = strrchr(fsname, '/');
+               p2 = strrchr(stream_fsname, '/');
+
                 if ((stream_parent_fromsnap_guid != 0 &&
                     stream_parent_fromsnap_guid !=  
parent_fromsnap_guid) ||
-                   strcmp(strrchr(fsname, '/'),
-                   strrchr(stream_fsname, '/')) != 0) {
+                   (p1 != NULL && p2 != NULL && strcmp (p1, p2) != 0)  
||
+                    ((p1 == NULL) ^ (p2 == NULL))) {
                         nvlist_t *parent;
                         char tryname[ZFS_MAXNAMELEN];

@@ -1317,7 +1322,7 @@
                                 VERIFY(0 ==  
nvlist_lookup_string(parent, "name",
                                     &pname));
                                 (void) snprintf(tryname, sizeof  
(tryname),
-                                   "%s%s", pname,  
strrchr(stream_fsname, '/'));
+                               "%s%s", pname, p2 ? p2 : "");
                         } else {
                                 tryname[0] = '\0';
                                 if (flags.verbose) {



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?09277772-9C54-4AE6-A147-CB6A4ED38C48>