Hi Alok, thank you very much for reviewing those changes ! Please see my comments in line. The webrev has been updated accordingly.
Jan On 04/09/09 17:57, Alok Aggarwal wrote: > Hi Jan, > > On Thu, 9 Apr 2009, jan damborsky wrote: > >> Hi, >> >> could I please ask two people for reviewing changes for following >> blocker ? >> >> 7944 auto-install dumps core installing 2009.06 b110 on T6300 >> http://defect.opensolaris.org/bz/show_bug.cgi?id=7944 >> >> webrev: >> http://cr.opensolaris.org/~dambi/bug-7944 > > Nice work in root causing this bug! Thank you. Kudos to Vitezslav Batrla from Prague RPE team who gave me quick "Sparc core dump analysis in an half an hour" course :-) > > target_discovery.c: line 897: I am wondering if this > is really a fatal condition and we should stop the > installation at this point instead. Otherwise I could > see we might get break the assumptions elsewhere in > the code about max slices <= NDKMAP. This is a good suggestion, I have changed the code, so that it exits with failure when this condition is met. > > target_discovery.c: line 894: Nit - "something might" -> > "something" Changed. > > td_mg.c: line 2058: Back tracking through the call chain > I can see that the passed in char *disk, is really a > disk_info_t.disk_name which is a char *. In other words > the disk_name is unbounded. In all likelihood it will > be <MAXPATHLEN, but what is the effect on target discovery > if it isn't (esp line 2097)? The assumption made here was based on following consideration: partition/slice device name is also part of path in /dev/ directory, e.g. /dev/dsk/<disk_name><slice_name> - and its total size is limited to MAXPATHLEN, since it is regular path. Since we are dealing only with '<disk_name><slice_name>' part here, its size has is always <MAXPATHLEN. If mangled/invalid disk name is provided, this is not right place to decide and take correct action, the only thing we could do here is to be robust and avoid buffer overflow which is assured by using snprintf. > > td_mg.c: line 2127: Could the comment be reworded as "Look for an > exact match between the slice/partition > (pobjname) we're processing and the passed in disk name > + disk part (eg: c0t0d0s or c0d0p)"? I have changed this.
