Re: disk_clone() bug

2001-10-22 Thread Poul-Henning Kamp

In message <[EMAIL PROTECTED]>, Chad David writes:
>On Fri, Oct 19, 2001 at 05:00:16PM +0200, Poul-Henning Kamp wrote:
>> 
>> Sounds like the bug is the md driver cloning "md10ec" which it shouldn't
>> do.  This bug must naturally be in md_clone(), but I don't have the
>> minutes right now to hunt it down.
>> 
>> Should be quite simple to nail, it's just some string handling code.

I've committed the correct fix.

Thanks!

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
[EMAIL PROTECTED] | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: disk_clone() bug

2001-10-19 Thread Chad David

On Fri, Oct 19, 2001 at 05:00:16PM +0200, Poul-Henning Kamp wrote:
> 
> Sounds like the bug is the md driver cloning "md10ec" which it shouldn't
> do.  This bug must naturally be in md_clone(), but I don't have the
> minutes right now to hunt it down.
> 
> Should be quite simple to nail, it's just some string handling code.

I might be missing something again, so please be kind :).

First, there is not such function md_clone(), so I assume you mean
disk_clone()?

My analysis of this is that devfs_lookupx() attempts to have the driver
create any files that it does not have nodes for:

EVENTHANDLER_INVOKE(dev_clone, pname, strlen(pname), &cdev);
on line 339 of devfs_vnops.c.

The invoke will eventually end up calling disk_clone() which will attempt
to create the device, and then devfs_lookupx() will happily make the file
visable.

The code in disk_clone() looks for a device name that matches the start of
the name we are looking up, and if it finds it tries to make sure the
remander of the name matches the proper disk device naming convention.
If it does end up matching then it creates the minor device with the name
given.  The problem is that it does not verify that the given name does not
contain any characters after the partition.  So instead of seeing md10ec it
only sees md10e, and passes it on to make_dev().  The real issue is that the
minor number for md10ec and md10e are the same, so of course make_dev() is
unhappy when makedev() returns an named dev_t.

My original patch silently cut off the last part of the file name, which is
probably not correct.  IMHO the code should fail first, ask questions later.
Dima's patch from around the end of July handles it properly I believe.

Something like this:

@@ -82,6 +82,11 @@
continue;
else
p = name[i] - 'a';
+
+   if (name[i + 1] != '\0') {
+   printf("WARNING: bad device name (\"%s\")\n", name);
+   return;
+   }
}
 
*dev = make_dev(pdev->si_devsw, dkmakeminor(u, s, p), 

Note that this has nothing to do with md.  Any device that is cloned by
disk_clone() will do the same thing.  Just doing an ls -l ad2eeec followed
by ls -l ad2e will print the same "Driver mistake" message.

-- 
Chad David[EMAIL PROTECTED]
ACNS Inc. Calgary, Alberta Canada

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: disk_clone() bug

2001-10-19 Thread Chad David

On Fri, Oct 19, 2001 at 08:24:57AM -0700, John Baldwin wrote:
> 
> On 19-Oct-01 Chad David wrote:
> > I posted a bug report and patch in kern/29104 and Dima Dorfman also mentioned
> > this in July/August, but it still has not been resolved.  The method of
> > triggering it that I detailed in my bug report no longer seems to work,
> > but I've managed to create another one.
> > 
> ># mdconfig -a -t swap -s 32m -u 10
> ># disklabel -r -w md10 auto
> ># disklabel -e md10e (copy c to e and set type to 4.2BSD)
> ># ls -l /dev/md10*
> > crw-r-  1 root  operator   95, 0x00010052 Oct 19 01:50 /dev/md10
> > crw-r-  1 root  operator   95,  82 Oct 19 02:00 /dev/md10c
> > crw-r-  1 root  operator   95,  84 Oct 19 01:55 /dev/md10ec
> > crw---  1 root  wheel  95, 0x00ff Oct 19 01:50 /dev/mdctl
> 
> Well, you are supposed to be running disklabel -e on md10, not md10e.  You edit
> the disklabel on a disk or slice, not the disklabel from inside of a partition
> defined by that disklabel.  Make sense? :)  Disklabel should probably fail to
> actually run in this case since md10e doesn't exist and certainly wouldn't have
> a valid disklabel to edit if it did exist.

Yes, that does make sense.  Thank you.  In normal cases disklabel would
have failed, but the (md) disk clone code seems to (incorrectly) create
it on the fly.  Even an ls /dev/md10e would create the file.  The problem
is that ls -l md10eeec returns

crw-r- 1 root operator 95, 20 Oct 19 02:19 /dev/md2eeec
which I'm guessing is a bad device name :).

Since phk has indicated where the real problem is I will see if I can
track it down and provide a patch.

Thanks.

-- 
Chad David[EMAIL PROTECTED]
ACNS Inc. Calgary, Alberta Canada
"When Linux was first ported to the Furby platform,
it suffered from significant stability and performance
problems, which gave the Furby an unfortunate reputation
as being unsuitable for enterprise-level computing."
 -- furbeowulf site


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



RE: disk_clone() bug

2001-10-19 Thread John Baldwin


On 19-Oct-01 Chad David wrote:
> I posted a bug report and patch in kern/29104 and Dima Dorfman also mentioned
> this in July/August, but it still has not been resolved.  The method of
> triggering it that I detailed in my bug report no longer seems to work,
> but I've managed to create another one.
> 
># mdconfig -a -t swap -s 32m -u 10
># disklabel -r -w md10 auto
># disklabel -e md10e (copy c to e and set type to 4.2BSD)
># ls -l /dev/md10*
> crw-r-  1 root  operator   95, 0x00010052 Oct 19 01:50 /dev/md10
> crw-r-  1 root  operator   95,  82 Oct 19 02:00 /dev/md10c
> crw-r-  1 root  operator   95,  84 Oct 19 01:55 /dev/md10ec
> crw---  1 root  wheel  95, 0x00ff Oct 19 01:50 /dev/mdctl

Well, you are supposed to be running disklabel -e on md10, not md10e.  You edit
the disklabel on a disk or slice, not the disklabel from inside of a partition
defined by that disklabel.  Make sense? :)  Disklabel should probably fail to
actually run in this case since md10e doesn't exist and certainly wouldn't have
a valid disklabel to edit if it did exist.

-- 

John Baldwin <[EMAIL PROTECTED]> -- http://www.FreeBSD.org/~jhb/
PGP Key: http://www.baldwin.cx/~john/pgpkey.asc
"Power Users Use the Power to Serve!"  -  http://www.FreeBSD.org/

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: disk_clone() bug

2001-10-19 Thread Poul-Henning Kamp


Sounds like the bug is the md driver cloning "md10ec" which it shouldn't
do.  This bug must naturally be in md_clone(), but I don't have the
minutes right now to hunt it down.

Should be quite simple to nail, it's just some string handling code.

Poul-Henning

In message <[EMAIL PROTECTED]>, Chad David writes:
>I posted a bug report and patch in kern/29104 and Dima Dorfman also mentioned
>this in July/August, but it still has not been resolved.  The method of
>triggering it that I detailed in my bug report no longer seems to work,
>but I've managed to create another one.
>
># mdconfig -a -t swap -s 32m -u 10
># disklabel -r -w md10 auto
># disklabel -e md10e (copy c to e and set type to 4.2BSD)
># ls -l /dev/md10*
>crw-r-  1 root  operator   95, 0x00010052 Oct 19 01:50 /dev/md10
>crw-r-  1 root  operator   95,  82 Oct 19 02:00 /dev/md10c
>crw-r-  1 root  operator   95,  84 Oct 19 01:55 /dev/md10ec
>crw---  1 root  wheel  95, 0x00ff Oct 19 01:50 /dev/mdctl
>
>Now if you access (open(2)) md10e the system will kill itself when you
>run mdconfig -d.  If I am doing something evil I'd like to know, but either
>way the kernel should not enter an endless loop!
>
>I have not had time to determine why disklabel -e md10e will kill it, but
>disklabel -e /dev/md10e will not.  I think it has to do with how disklabel
>appends the 'c'.
>
>The patch that I provided in 29104 is probably not the correct solution
>(I like Dima's default better), and may not even be "fixing" the problem
>in the correct place.  Any advice would be very welcome.
>
>-- 
>Chad David[EMAIL PROTECTED]
>ACNS Inc. Calgary, Alberta Canada
>
>To Unsubscribe: send mail to [EMAIL PROTECTED]
>with "unsubscribe freebsd-current" in the body of the message
>

-- 
Poul-Henning Kamp   | UNIX since Zilog Zeus 3.20
[EMAIL PROTECTED] | TCP/IP since RFC 956
FreeBSD committer   | BSD since 4.3-tahoe
Never attribute to malice what can adequately be explained by incompetence.

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message