Re: Crash on -current on i386 (NOT amd64)

[John Nemeth <jnemeth%cue.bc.ca@localhost>]

On Dec 5,  7:56am, Christos Zoulas wrote:
} On Dec 5,  3:30pm, paul%vps1.whooppee.com@localhost (Paul Goyette) wrote:
} 
} | > } The same sources were used to create a amd64-current system, and I was
} | > } able to install it successfully.
} | > } 
} | > } FWIW, my source tree is time-stamped at "2015-12-05 at 03:08:57 UTC"
} | > }
} | > } The failure occurs when trying to mount the (qemu-emulated) cd-rom
} | > } containing the release files.
} | >
} | >     Try this patch:
} | >
} | > --- sys/miscfs/specfs/spec_vnops.c.orig 2015-12-04 22:26:43.000000000 -0800
} | > +++ sys/miscfs/specfs/spec_vnops.c      2015-12-04 22:28:10.000000000 -0800
} | > @@ -668,7 +668,8 @@
} | >                printf("ioctl DIOCGMEDIASIZE failed %d\n", error);
} | > #endif
} | >                error = (*ioctl)(vp->v_rdev, DIOCGPART, &pi, FREAD, curlwp);
} | > -               off = (off_t)pi.disklab->d_secsize * pi.part->p_size;
} | > +               if (error == 0)
} | > +                       off = (off_t)pi.disklab->d_secsize * pi.part->p_size;
} | >        }
} | >
} | >        if (error == 0)
} | 
} | I have built a new release with this patch, and sysinst will now
} | complete the installation successfully. And the newly installed system
} | also boots successfully.
} | 
} 
} What's the backtrace? Ideally I'd like to remove the DIOCGPART code...

     I don't think there was a backtrace.  I saw your comment on
chat that you had fixed the lseek(SEEK_END) issue, and was curious
what it was, so I looked for the commit, and spotted the bug.  I
then saw Paul's message about his crash about mounting a CD and
noted your new DIAGNOSTIC message about DIOCGMEDIASIZE failing in
his output, which hinted that it was your patch.  At first, I was
going to suggest that he revert it, but then decided that I could
fix it (I don't know anything about file system code, so normally
don't go anywhere near it).

     The issues goes like this.  The original code in pseudo code
format (with irrelevant details omitted goes like this):

1) ioctl(DIOCGPART)
2) if successful
3)   use struct partinfo to determine disk size
4)   call uvm_vnp_setsize(disk size)

Your new code did:

1) ioctl(DIOCGMEDIASIZE)
2) if failure
3)   diagnostic(DIOCGMEDIASIZE failed)
4)   ioctl(DIOCGPART)
5)   use struct partinfo to determine disk size
6) if either ioctl was successful
7)   call uvm_vnp_setsize(disk size)

Note that at step 5, you were dereferencing two pointers in struct
partinfo regardless of whether or not the ioctl(DIOCGPART) was
successful, which is what initializes the struct partinfo.  I just
did the obvious and inserted step 4.5, "if successful", to avoid
dereferencing NULL pointers.

     The implication of all this is that the CD driver in question
doesn't support either DIOCGMEDIASIZE or DIOCGPART.  Thus for the
CD driver, there is no functional difference between the original
code and your new code (with my fix).

}-- End of excerpt from Christos Zoulas