* mdadm-2.2 SEGFAULT: mdadm --assemble --scan @ 2005-12-22 20:58 Andre Noll 2005-12-31 15:17 ` Andreas Haumer 0 siblings, 1 reply; 5+ messages in thread From: Andre Noll @ 2005-12-22 20:58 UTC (permalink / raw) To: linux-raid; +Cc: Neil Brown sorry if this is already known/fixed: Assemble() is called from mdadm.c with the "update" argument equal to NULL: Assemble(ss, array_list->devname, mdfd, array_list, configfile, NULL, readonly, runstop, NULL, verbose-quiet, force); But in Assemble.c we have if (ident->uuid_set && (!update && strcmp(update, "uuid")!= 0) && ... which yields a segfault in glibc's strcmp(). Andre -- Jesus not only saves, he also frequently makes backups ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: mdadm-2.2 SEGFAULT: mdadm --assemble --scan 2005-12-22 20:58 mdadm-2.2 SEGFAULT: mdadm --assemble --scan Andre Noll @ 2005-12-31 15:17 ` Andreas Haumer 2006-01-08 21:23 ` Bill Davidsen 0 siblings, 1 reply; 5+ messages in thread From: Andreas Haumer @ 2005-12-31 15:17 UTC (permalink / raw) To: Andre Noll; +Cc: linux-raid, Neil Brown [-- Attachment #1: Type: text/plain, Size: 1198 bytes --] -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi! Andre Noll schrieb: > sorry if this is already known/fixed: Assemble() is called from mdadm.c with > the "update" argument equal to NULL: > > Assemble(ss, array_list->devname, mdfd, array_list, configfile, > NULL, readonly, runstop, NULL, verbose-quiet, force); > > But in Assemble.c we have > > if (ident->uuid_set && (!update && strcmp(update, "uuid")!= 0) && ... > > which yields a segfault in glibc's strcmp(). > I just found the same problem after upgrading to mdadm-2.2 The logic to test for update not being NULL seems to be reversed. I created a small patch which seems to cure the problem (see attached file) HTH - - andreas - -- Andreas Haumer | mailto:andreas@xss.co.at *x Software + Systeme | http://www.xss.co.at/ Karmarschgasse 51/2/20 | Tel: +43-1-6060114-0 A-1100 Vienna, Austria | Fax: +43-1-6060114-71 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDtqD0xJmyeGcXPhERAsdiAJ0Ve787gscq4VOGtT+9Qp3k62iUEgCgs9pH Ekg0gkLEk+99XXHw+1ezdu8= =rh66 -----END PGP SIGNATURE----- [-- Attachment #2: update_uuid.patch --] [-- Type: text/plain, Size: 697 bytes --] Index: mdadm/Assemble.c =================================================================== RCS file: /home/cvs/repository/distribution/Utilities/mdadm/Assemble.c,v retrieving revision 1.1.1.7 diff -u -r1.1.1.7 Assemble.c --- mdadm/Assemble.c 5 Dec 2005 05:56:20 -0000 1.1.1.7 +++ mdadm/Assemble.c 31 Dec 2005 15:01:34 -0000 @@ -219,7 +219,7 @@ } if (dfd >= 0) close(dfd); - if (ident->uuid_set && (!update && strcmp(update, "uuid")!= 0) && + if (ident->uuid_set && (update && strcmp(update, "uuid")!= 0) && (!super || same_uuid(info.uuid, ident->uuid, tst->ss->swapuuid)==0)) { if ((inargv && verbose >= 0) || verbose > 0) fprintf(stderr, Name ": %s has wrong uuid.\n", ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: mdadm-2.2 SEGFAULT: mdadm --assemble --scan 2005-12-31 15:17 ` Andreas Haumer @ 2006-01-08 21:23 ` Bill Davidsen 2006-01-11 13:51 ` Andreas Haumer 0 siblings, 1 reply; 5+ messages in thread From: Bill Davidsen @ 2006-01-08 21:23 UTC (permalink / raw) To: Andreas Haumer; +Cc: Andre Noll, linux-raid, Neil Brown Andreas Haumer wrote: >-----BEGIN PGP SIGNED MESSAGE----- >Hash: SHA1 > >Hi! > >Andre Noll schrieb: > > >>sorry if this is already known/fixed: Assemble() is called from mdadm.c with >>the "update" argument equal to NULL: >> >> Assemble(ss, array_list->devname, mdfd, array_list, configfile, >> NULL, readonly, runstop, NULL, verbose-quiet, force); >> >>But in Assemble.c we have >> >> if (ident->uuid_set && (!update && strcmp(update, "uuid")!= 0) && ... >> >>which yields a segfault in glibc's strcmp(). >> >> >> >I just found the same problem after upgrading to mdadm-2.2 >The logic to test for update not being NULL seems to be >reversed. > >I created a small patch which seems to cure the problem >(see attached file) > >HTH > >- - andreas > >- -- >Andreas Haumer | mailto:andreas@xss.co.at >*x Software + Systeme | http://www.xss.co.at/ >Karmarschgasse 51/2/20 | Tel: +43-1-6060114-0 >A-1100 Vienna, Austria | Fax: +43-1-6060114-71 >-----BEGIN PGP SIGNATURE----- >Version: GnuPG v1.4.2 (GNU/Linux) >Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org > >iD8DBQFDtqD0xJmyeGcXPhERAsdiAJ0Ve787gscq4VOGtT+9Qp3k62iUEgCgs9pH >Ekg0gkLEk+99XXHw+1ezdu8= >=rh66 >-----END PGP SIGNATURE----- > > >------------------------------------------------------------------------ > >Index: mdadm/Assemble.c >=================================================================== >RCS file: /home/cvs/repository/distribution/Utilities/mdadm/Assemble.c,v >retrieving revision 1.1.1.7 >diff -u -r1.1.1.7 Assemble.c >--- mdadm/Assemble.c 5 Dec 2005 05:56:20 -0000 1.1.1.7 >+++ mdadm/Assemble.c 31 Dec 2005 15:01:34 -0000 >@@ -219,7 +219,7 @@ > } > if (dfd >= 0) close(dfd); > >- if (ident->uuid_set && (!update && strcmp(update, "uuid")!= 0) && >+ if (ident->uuid_set && (update && strcmp(update, "uuid")!= 0) && > (!super || same_uuid(info.uuid, ident->uuid, tst->ss->swapuuid)==0)) { > if ((inargv && verbose >= 0) || verbose > 0) > fprintf(stderr, Name ": %s has wrong uuid.\n", > > Is that right now? Because && evaluates to zero or one left to right, the parens and the "!=0" are not needed, and I assume they're in for a reason (other than to make the code hard to understand). A comment before that if would make the intention clear, I originally though the "(!update" was intended to be "!(update" which would explain the parens, but that seems wrong. If it actually works as intended with the patch, perhaps a comment and cleanup in 2.3? -- bill davidsen <davidsen@tmr.com> CTO TMR Associates, Inc Doing interesting things with small computers since 1979 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: mdadm-2.2 SEGFAULT: mdadm --assemble --scan 2006-01-08 21:23 ` Bill Davidsen @ 2006-01-11 13:51 ` Andreas Haumer 2006-01-24 6:28 ` Neil Brown 0 siblings, 1 reply; 5+ messages in thread From: Andreas Haumer @ 2006-01-11 13:51 UTC (permalink / raw) To: Bill Davidsen; +Cc: Andre Noll, linux-raid, Neil Brown -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 Hi! Bill Davidsen schrieb: > Andreas Haumer wrote: > - ------------------------------------------------------------------------ >> Index: mdadm/Assemble.c =================================================================== RCS file: /home/cvs/repository/distribution/Utilities/mdadm/Assemble.c,v retrieving revision 1.1.1.7 diff -u -r1.1.1.7 Assemble.c - --- mdadm/Assemble.c 5 Dec 2005 05:56:20 -0000 1.1.1.7 +++ mdadm/Assemble.c 31 Dec 2005 15:01:34 -0000 @@ -219,7 +219,7 @@ } if (dfd >= 0) close(dfd); >> - - if (ident->uuid_set && (!update && strcmp(update, "uuid")!= 0) && + if (ident->uuid_set && (update && strcmp(update, "uuid")!= 0) && (!super || same_uuid(info.uuid, ident->uuid, tst->ss->swapuuid)==0)) { if ((inargv && verbose >= 0) || verbose > 0) fprintf(stderr, Name ": %s has wrong uuid.\n", >> > Is that right now? Because && evaluates to zero or one left to right, > the parens and the "!=0" are not needed, and I assume they're in for a > reason (other than to make the code hard to understand). A comment > before that if would make the intention clear, I originally though the > "(!update" was intended to be "!(update" which would explain the parens, > but that seems wrong. I made this modification out of the following reasoning: It does not make sense to check if update is NULL and then use it in a strcmp(). It only makes sense to check if update is _not_ NULL and then do the strcmp() (a similar code fragment can be found in the same sourcefile several lines below) This cures the segfault, but I can not really say if the whole construct is logically correct (you are right, it looks suspicious...) That should be answered by Neil ;-) - - andreas - -- Andreas Haumer | mailto:andreas@xss.co.at *x Software + Systeme | http://www.xss.co.at/ Karmarschgasse 51/2/20 | Tel: +43-1-6060114-0 A-1100 Vienna, Austria | Fax: +43-1-6060114-71 -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2 (GNU/Linux) Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org iD8DBQFDxQ16xJmyeGcXPhERAs2fAJ44Pta06tMd6yI5HqXuRvYkKbWHSACfcPjD y+b0ddT/ezuqf+rHurm2+Wo= =SkuC -----END PGP SIGNATURE----- ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: mdadm-2.2 SEGFAULT: mdadm --assemble --scan 2006-01-11 13:51 ` Andreas Haumer @ 2006-01-24 6:28 ` Neil Brown 0 siblings, 0 replies; 5+ messages in thread From: Neil Brown @ 2006-01-24 6:28 UTC (permalink / raw) To: Andreas Haumer; +Cc: Bill Davidsen, Andre Noll, linux-raid On Wednesday January 11, andreas@xss.co.at wrote: > Index: mdadm/Assemble.c > =================================================================== > RCS file: /home/cvs/repository/distribution/Utilities/mdadm/Assemble.c,v > retrieving revision 1.1.1.7 > diff -u -r1.1.1.7 Assemble.c > - --- mdadm/Assemble.c 5 Dec 2005 05:56:20 -0000 1.1.1.7 > +++ mdadm/Assemble.c 31 Dec 2005 15:01:34 -0000 > @@ -219,7 +219,7 @@ > } > if (dfd >= 0) close(dfd); > >> > - - if (ident->uuid_set && (!update && strcmp(update, "uuid")!= > 0) && > + if (ident->uuid_set && (update && strcmp(update, "uuid")!= 0) && > (!super || same_uuid(info.uuid, ident->uuid, > tst->ss->swapuuid)==0)) { > if ((inargv && verbose >= 0) || verbose > 0) > fprintf(stderr, Name ": %s has wrong uuid.\n", > > >> > > Is that right now? Because && evaluates to zero or one left to right, > > the parens and the "!=0" are not needed, and I assume they're in for a > > reason (other than to make the code hard to understand). A comment > > before that if would make the intention clear, I originally though the > > "(!update" was intended to be "!(update" which would explain the parens, > > but that seems wrong. > > I made this modification out of the following reasoning: > > It does not make sense to check if update is NULL and > then use it in a strcmp(). It only makes sense to check > if update is _not_ NULL and then do the strcmp() > > (a similar code fragment can be found in the same sourcefile > several lines below) > > This cures the segfault, but I can not really say if the > whole construct is logically correct (you are right, it > looks suspicious...) > That should be answered by Neil ;-) Sorry it has taken me so long to get back to you on this... Thanks for reporting the problem. The correct fix is diff ./Assemble.c~current~ ./Assemble.c --- ./Assemble.c~current~ 2005-12-05 16:56:20.000000000 +1100 +++ ./Assemble.c 2006-01-24 17:25:20.000000000 +1100 @@ -219,7 +219,7 @@ int Assemble(struct supertype *st, char } if (dfd >= 0) close(dfd); - if (ident->uuid_set && (!update && strcmp(update, "uuid")!= 0) && + if (ident->uuid_set && (!update || strcmp(update, "uuid")!= 0) && (!super || same_uuid(info.uuid, ident->uuid, tst->ss->swapuuid)==0)) { if ((inargv && verbose >= 0) || verbose > 0) fprintf(stderr, Name ": %s has wrong uuid.\n", i.e. change the && to and ||. I guess I'd better do a '2.3'... Thanks, NeilBrown ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2006-01-24 6:28 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2005-12-22 20:58 mdadm-2.2 SEGFAULT: mdadm --assemble --scan Andre Noll 2005-12-31 15:17 ` Andreas Haumer 2006-01-08 21:23 ` Bill Davidsen 2006-01-11 13:51 ` Andreas Haumer 2006-01-24 6:28 ` Neil Brown
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).