fix pktcdvd breakage from commit e105b8bfc769b0545b6f0f395179d1e43cbee822

PATCH In review Al Viro 3 participants 18 messages Nov 30, 2008 – Dec 9, 2008 Block layer

Share & tools

Al Viro

author

2008-11-30 12:19 UTC #0 lore raw

PATCH 1/1 fix pktcdvd breakage from commit e105b8bfc769b0545b6f0f395179d1e43cbee822

device_create() will create a symlink in /sys/dev/char unless
the class has ->dev_kobj cleared; pktcdvd uses it to populate the /sys/class/pktcdvd/ with per-device subdirectories. While we do want /sys/class/pktcdvd/pktcdvd[0-7]/dev to contain a device number, we definitely do not want it to try and crap into /sys/dev/char; if nothing else, device number is *block* one (and we don't want it to crap into /sys/dev/block either - there such symlinks will be created by add_disk() and they will point to /sys/block/pktcdvd[0-7]).

As it is, attempt to set the damn thing up will end up with
sysfs_add_one() barfing at duplicate entries and /sys/class/pktcdvd/pktcdvd*/*/ not created at all. The fix is trivial: clear ->dev_kobj in the class to tell device_add() that no, we do *not* want these symlinks, TYVM...

FWIW, all traces containing pkt_setup_dev() on kerneloops.org appear
to be from that one. It had been around since 2.6.26; probably -stable fodder...

Signed-off-by Al Viro <[email protected]>

---

drivers/block/pktcdvd.c +1 -0

index f20bf35..79981f2 100644
@@ -417,6 +417,7 @@ static int pkt_sysfs_init(void)
 		printk(DRIVER_NAME": failed to create class pktcdvd\n");
 		return ret;
 	}
+	class_pktcdvd->dev_kobj = NULL;
 	return 0;
 }

Kay Sievers

2008-11-30 12:41 UTC #1 lore raw

Quoted 25 lines · Al Viro

device_create() will create a symlink in /sys/dev/char unless
the class has ->dev_kobj cleared; pktcdvd uses it to populate the /sys/class/pktcdvd/ with per-device subdirectories. While we do want /sys/class/pktcdvd/pktcdvd[0-7]/dev to contain a device number, we definitely do not want it to try and crap into /sys/dev/char; if nothing else, device number is *block* one (and we don't want it to crap into /sys/dev/block either - there such symlinks will be created by add_disk() and they will point to /sys/block/pktcdvd[0-7]).
As it is, attempt to set the damn thing up will end up with
sysfs_add_one() barfing at duplicate entries and /sys/class/pktcdvd/pktcdvd*/*/ not created at all. The fix is trivial: clear ->dev_kobj in the class to tell device_add() that no, we do *not* want these symlinks, TYVM...
FWIW, all traces containing pkt_setup_dev() on kerneloops.org appear
to be from that one. It had been around since 2.6.26; probably -stable fodder...
Signed-off-by Al Viro <[email protected]>
---
drivers/block/pktcdvd.c +0 -0
index f20bf35..79981f2 100644

drivers/block/pktcdvd.c static int pkt_sysfs_init(void) · line 422

Quoted 6 lines

drivers/block/pktcdvd.c +1 -0

@@ -417,6 +417,7 @@ static int pkt_sysfs_init(void)
               printk(DRIVER_NAME": failed to create class pktcdvd\n");
               return ret;
       }
+       class_pktcdvd->dev_kobj = NULL;
       return 0;
 }

I posted a fix for that weeks ago. But the pktcdvd maintainer stated, that the char device nodes are not used for anything. So the whole use of dev_t should be removed entirely. They just blindly claim the same char dev_t the block devices use, and conflict with char devices from other subsystems. Patching out the /sys/dev/class links fixes the oops, but the underlying fundamental breakage will still exist.

Original mail here states:
"Maybe, but that character device would not be used for anything, besides creating sub-directories in /sys/class/pktcdvd. The driver implements a block device, not a character device."

http://lkml.org/lkml/2008/10/14/318

No idea why this never got fixed.

Thanks,
Kay

Al Viro

author

2008-11-30 13:21 UTC #2 lore raw

Quoted 11 lines · Kay Sievers

I posted a fix for that weeks ago. But the pktcdvd maintainer stated, that the char device nodes are not used for anything. So the whole use of dev_t should be removed entirely. They just blindly claim the same char dev_t the block devices use, and conflict with char devices from other subsystems. Patching out the /sys/dev/class links fixes the oops, but the underlying fundamental breakage will still exist.
Original mail here states:
"Maybe, but that character device would not be used for anything, besides creating sub-directories in /sys/class/pktcdvd. The driver implements a block device, not a character device."

_What_ character device nodes? The only thing that dev_t value used to be used for was giving contents for a text file - ..../dev in sysfs directory. With introduction of /sys/dev/char the damn thing got confused for character device number *by* *device_create()*. That's all.

There are no character device nodes at all. Driver doesn't claim them. Not in that call of device_create(), not anywhere else. sysfs doesn't create them (or any other device nodes). If you mknod such thing as char device and try to open it, the kernel won't go anywhere near this driver, whether there'd been other drivers or not.

So what kind of fundamental breakage are you talking about? All I can see here is a bogus entry added to bogu^Wmisguided^W /sys/dev/char, which leads to WARN_ON() in case if somebody had char device with device number numerically equal to that of our block device and to silent crapping into /sys/dev/char if no such char device had been there already.

Kay Sievers

2008-11-30 13:25 UTC #3 lore raw

Quoted 25 lines · Al Viro

Quoted 11 lines · Kay Sievers
I posted a fix for that weeks ago. But the pktcdvd maintainer stated, that the char device nodes are not used for anything. So the whole use of dev_t should be removed entirely. They just blindly claim the same char dev_t the block devices use, and conflict with char devices from other subsystems. Patching out the /sys/dev/class links fixes the oops, but the underlying fundamental breakage will still exist.
Original mail here states:
"Maybe, but that character device would not be used for anything, besides creating sub-directories in /sys/class/pktcdvd. The driver implements a block device, not a character device."
_What_ character device nodes? The only thing that dev_t value used to be used for was giving contents for a text file - ..../dev in sysfs directory. With introduction of /sys/dev/char the damn thing got confused for character device number *by* *device_create()*. That's all.
There are no character device nodes at all. Driver doesn't claim them. Not in that call of device_create(), not anywhere else. sysfs doesn't create them (or any other device nodes). If you mknod such thing as char device and try to open it, the kernel won't go anywhere near this driver, whether there'd been other drivers or not.

Quoted 5 lines

So what kind of fundamental breakage are you talking about? All I can see here is a bogus entry added to bogu^Wmisguided^W /sys/dev/char, which leads to WARN_ON() in case if somebody had char device with device number numerically equal to that of our block device and to silent crapping into /sys/dev/char if no such char device had been there already

Pktcdvd creates char _and_ block device nodes at the same time, while the char nodes are not allocated, but created and conflict in /sys/dev/ with properly allocated ones from other subsystems. Your patch just papers over this bug.

Kay

Al Viro

author

2008-11-30 13:32 UTC #4 lore raw

Quoted 4 lines · Kay Sievers

Pktcdvd creates char _and_ block device nodes at the same time, while the char nodes are not allocated, but created and conflict in /sys/dev/ with properly allocated ones from other subsystems. Your patch just papers over this bug.

Where the hell is it creating any char device nodes? Show me. And note that device_create() is not creating *any* device nodes, char or block. It creates a directory in some place in sysfs, associated with struct device (which has arseloads of uses unrelated to block *or* char devices) and, if dev_t argument is non-zero, dumps its value into one of the attributes.

These days it might also dump a symlink into /sys/dev/*.

Which part of the above constitutes a creation of char device node?

Al Viro

author

2008-11-30 13:41 UTC #5 lore raw

Quoted 14 lines · Al Viro

Quoted 4 lines · Kay Sievers
Pktcdvd creates char _and_ block device nodes at the same time, while the char nodes are not allocated, but created and conflict in /sys/dev/ with properly allocated ones from other subsystems. Your patch just papers over this bug.
Where the hell is it creating any char device nodes? Show me. And note that device_create() is not creating *any* device nodes, char or block. It creates a directory in some place in sysfs, associated with struct device (which has arseloads of uses unrelated to block *or* char devices) and, if dev_t argument is non-zero, dumps its value into one of the attributes.

Quoted 3 lines

These days it might also dump a symlink into /sys/dev/*.
Which part of the above constitutes a creation of char device node?

Wait a minute... Are you saying that something in userland ends up seeing that sucker, noticing .../dev and proceeding to do mknod?

Kay Sievers

2008-11-30 13:44 UTC #6 lore raw

Quoted 15 lines · Al Viro

Quoted 14 lines · Al Viro
Quoted 4 lines · Kay Sievers
Pktcdvd creates char _and_ block device nodes at the same time, while the char nodes are not allocated, but created and conflict in /sys/dev/ with properly allocated ones from other subsystems. Your patch just papers over this bug.
Where the hell is it creating any char device nodes? Show me. And note that device_create() is not creating *any* device nodes, char or block. It creates a directory in some place in sysfs, associated with struct device (which has arseloads of uses unrelated to block *or* char devices) and, if dev_t argument is non-zero, dumps its value into one of the attributes.

Quoted 6 lines

Quoted 3 lines
These days it might also dump a symlink into /sys/dev/*.
Which part of the above constitutes a creation of char device node?
Wait a minute... Are you saying that something in userland ends up seeing that sucker, noticing .../dev and proceeding to do mknod?

Sure, it does mkod, as the kernel says it should. But that does not cause any real problem.

These device nodes are not used for anything. Opening the created char pktcdvd device node in userspace would in many cases just open a random usb device. :)

Kay

Al Viro

author

2008-11-30 13:50 UTC #7 lore raw

Quoted 7 lines · Kay Sievers

Quoted 4 lines
Quoted 1 line
Which part of the above constitutes a creation of char device node?
Wait a minute... Are you saying that something in userland ends up seeing that sucker, noticing .../dev and proceeding to do mknod?
Sure, it does mkod, as the kernel says it should. But that does not cause any real problem.

"It" being udev, presumably?

What a mess... How does drivers/usb/core/devio.c avoid essentially the same problem?

Kay Sievers

2008-11-30 13:57 UTC #8 lore raw

Quoted 11 lines · Al Viro

Quoted 7 lines · Kay Sievers
Quoted 4 lines
Quoted 1 line
Which part of the above constitutes a creation of char device node?
Wait a minute... Are you saying that something in userland ends up seeing that sucker, noticing .../dev and proceeding to do mknod?
Sure, it does mkod, as the kernel says it should. But that does not cause any real problem.
"It" being udev, presumably?

Yes, udev, mdev, and a few other tools people use to populate /dev from the kernel supplied device information.

Quoted 2 lines

What a mess... How does drivers/usb/core/devio.c avoid essentially the same problem?

It's a special case, where two "struct device" have the same dev_t, but they both point to and handle the same device, so it's fine. The usb_device class is deprecated, no recent distro uses it, and will be removed some day.

The /sys/dev/ is handled properly by doing:
usb_classdev_class->dev_kobj = NULL;

Kay

Al Viro

author

2008-11-30 14:13 UTC #9 lore raw

Quoted 5 lines · Kay Sievers

Quoted 1 line
"It" being udev, presumably?
Yes, udev, mdev, and a few other tools people use to populate /dev from the kernel supplied device information.

Quoted 7 lines

Quoted 2 lines
What a mess... How does drivers/usb/core/devio.c avoid essentially the same problem?
It's a special case, where two "struct device" have the same dev_t, but they both point to and handle the same device, so it's fine. The usb_device class is deprecated, no recent distro uses it, and will be removed some day.

Would that happened to udev as well... Yeah, I know, but one can dream ;-/

Bloody wonderful. So we have
* userland-exposed layout of directory trees created by pktcdvd
* uevent mess generated by device_create() and *also* userland-exposed

So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...

Kay Sievers

2008-11-30 14:28 UTC #10 lore raw

Quoted 9 lines · Al Viro

Quoted 8 lines · Kay Sievers
Quoted 1 line
"It" being udev, presumably?
Yes, udev, mdev, and a few other tools people use to populate /dev from the kernel supplied device information.
Quoted 2 lines
What a mess... How does drivers/usb/core/devio.c avoid essentially the same problem?

Quoted 6 lines

Quoted 4 lines
It's a special case, where two "struct device" have the same dev_t, but they both point to and handle the same device, so it's fine. The usb_device class is deprecated, no recent distro uses it, and will be removed some day.
Would that happened to udev as well... Yeah, I know, but one can dream ;-/

If there are two devices with the same dev_t, and udev is configured to create the same names in /dev, it will create the node for the first event, and reuse is for the second identical event. There is no problem here.

Quoted 7 lines

Bloody wonderful. So we have
* userland-exposed layout of directory trees created by pktcdvd
* uevent mess generated by device_create() and *also* userland-exposed
So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...

What do you mean? We just need to replace the bogus "pd->pkt_dev" with MKDEV(0, 0) and we are fine.

Kay

Al Viro

author

2008-11-30 14:52 UTC #11 lore raw

Quoted 6 lines · Kay Sievers

Quoted 3 lines
So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...
What do you mean? We just need to replace the bogus "pd->pkt_dev" with MKDEV(0, 0) and we are fine.

Userland-visible change - right now cat /sys/class/pktcdvd/pktcdvd3/dev will give you dev_t of the block device in question.

Al Viro

author

2008-11-30 15:53 UTC #12 lore raw

Quoted 10 lines · Al Viro

Quoted 6 lines · Kay Sievers
Quoted 3 lines
So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...
What do you mean? We just need to replace the bogus "pd->pkt_dev" with MKDEV(0, 0) and we are fine.
Userland-visible change - right now cat /sys/class/pktcdvd/pktcdvd3/dev will give you dev_t of the block device in question.

Looking at the locking scheme there, it appears that we have an unpleasant race of the same kind as discussed in md-with-eviction thread. If removal hits between finding (and grabbing) gendisk and actual call of ->open(), it will succeed just fine and we might get an open for _different_ object, while holding a reference to disk that had already gone through del_gendisk(). Call ioctl() on that sucker and you've got

        struct pktcdvd_device *pd = bdev->bd_disk->private_data;

which will point to already freed object.

It really appears that we need to
* add ->use() and ->unuse() callbacks
* have exact_lock() call ->use() in addition to get_disk() - either
on each hit, or on the "it's currently not in use" ones.
* have ->unuse() called on failure exits in __blkdev_get() and in
blkdev_put(), with rules matching those for calls of ->use().

That would allow to close that kind of races, both here and in md. I'd probably prefer the second variant for calling rules - we would be able to bracket the "it's associated with underlying objects" intervals by those calls... Hell knows; I really need to get some sleep and look through the ->open() and ->release() instances for block devices. Later...

Kay Sievers

2008-11-30 15:56 UTC #13 lore raw

In reply to Al Viro

Quoted 10 lines · Al Viro

Quoted 6 lines · Kay Sievers
Quoted 3 lines
So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...
What do you mean? We just need to replace the bogus "pd->pkt_dev" with MKDEV(0, 0) and we are fine.
Userland-visible change - right now cat /sys/class/pktcdvd/pktcdvd3/dev will give you dev_t of the block device in question.

True, it's visible, but it's incorrect dead information which should be removed. Subsystem != "block is defined as S_IFCHR, and there is no such char device for pktcdvd. In fact the "dev" file of the pktcdvd class device points in many cases to a USB device, which must be fixed.

The whole pktcdvd class is not used at all, besides for exporting a few files. The "dev" file at the class device is just a bug, and the MAJOR/MINOR uevent environment variables also. I doubt that there will be any visible breakage in something that isn't already totally broken. In short, I doubt, that anything that works today will stop working if we remove the bugus "dev" file.

This seem like the simple and correct fix for the issue with /sys/dev/ and the broken non-existing but exported pkcdvd char device:
- pd->dev = device_create(class_pktcdvd, NULL, pd->pkt_dev, NULL,
+ pd->dev = device_create(class_pktcdvd, NULL, MKDEV(0, 0), NULL,

Kay

Kay Sievers

2008-12-05 02:57 UTC #14 lore raw

Quoted 27 lines · Kay Sievers

Quoted 10 lines · Al Viro
Quoted 6 lines · Kay Sievers
Quoted 3 lines
So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...
What do you mean? We just need to replace the bogus "pd->pkt_dev" with MKDEV(0, 0) and we are fine.
Userland-visible change - right now cat /sys/class/pktcdvd/pktcdvd3/dev will give you dev_t of the block device in question.
True, it's visible, but it's incorrect dead information which should be removed. Subsystem != "block is defined as S_IFCHR, and there is no such char device for pktcdvd. In fact the "dev" file of the pktcdvd class device points in many cases to a USB device, which must be fixed.
The whole pktcdvd class is not used at all, besides for exporting a few files. The "dev" file at the class device is just a bug, and the MAJOR/MINOR uevent environment variables also. I doubt that there will be any visible breakage in something that isn't already totally broken. In short, I doubt, that anything that works today will stop working if we remove the bugus "dev" file.
This seem like the simple and correct fix for the issue with /sys/dev/ and the broken non-existing but exported pkcdvd char device:

Review

Quoted 2 lines

(quoted diff) +1 -1

-               pd->dev = device_create(class_pktcdvd, NULL, pd->pkt_dev, NULL,
+               pd->dev = device_create(class_pktcdvd, NULL, MKDEV(0, 0), NULL,

Al, this bug still exists, right? Any objections to remove the dead char dev_t?

Thanks,
Kay

Kay Sievers

2008-12-06 03:38 UTC #15 lore raw

Quoted 26 lines · Kay Sievers

Quoted 25 lines · Kay Sievers
Quoted 10 lines · Al Viro
Quoted 6 lines · Kay Sievers
Quoted 3 lines
So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...
What do you mean? We just need to replace the bogus "pd->pkt_dev" with MKDEV(0, 0) and we are fine.
Userland-visible change - right now cat /sys/class/pktcdvd/pktcdvd3/dev will give you dev_t of the block device in question.
True, it's visible, but it's incorrect dead information which should be removed. Subsystem != "block is defined as S_IFCHR, and there is no such char device for pktcdvd. In fact the "dev" file of the pktcdvd class device points in many cases to a USB device, which must be fixed.
The whole pktcdvd class is not used at all, besides for exporting a few files. The "dev" file at the class device is just a bug, and the MAJOR/MINOR uevent environment variables also. I doubt that there will be any visible breakage in something that isn't already totally broken. In short, I doubt, that anything that works today will stop working if we remove the bugus "dev" file.

Quoted 6 lines

Review
Quoted 4 lines
This seem like the simple and correct fix for the issue with /sys/dev/ and the broken non-existing but exported pkcdvd char device:
(quoted diff) +1 -1
-               pd->dev = device_create(class_pktcdvd, NULL, pd->pkt_dev, NULL,
+               pd->dev = device_create(class_pktcdvd, NULL, MKDEV(0, 0), NULL,
Al, this bug still exists, right? Any objections to remove the dead char dev_t?

Here is a patch.

Thanks,
Kay

From: Kay Sievers <[email protected]>
Subject: pktcdvd: remove broken dev_t export of class devices

The pktcdvd created class devices only export some sysfs files, but have no char dev_t registered in the driver.

At class device creation time they copy the dev_t value of the block device to the char device, wich will register a new char device in the driver core and userspace, with a conflicting dev_t value.

In many cases the class devices dev_t just points to a random USB device. This fixes the sysfs "duplicate entry" errors.

Cc [email protected]

Cc Al Viro <[email protected]>

Signed-off-by Kay Sievers <[email protected]>

---
 pktcdvd.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/block/pktcdvd.c +1 -1

@@ -302,7 +302,7 @@ static struct kobj_type kobj_pkt_type_wqueue = {
 static void pkt_sysfs_dev_new(struct pktcdvd_device *pd)
 {
 	if (class_pktcdvd) {
-		pd->dev = device_create(class_pktcdvd, NULL, pd->pkt_dev, NULL,
+		pd->dev = device_create(class_pktcdvd, NULL, MKDEV(0, 0), NULL,
 					"%s", pd->name);
 		if (IS_ERR(pd->dev))
 			pd->dev = NULL;

Peter Osterlund

Acked-by

2008-12-09 23:07 UTC #16 lore raw

Quoted 27 lines · Kay Sievers

Quoted 26 lines · Kay Sievers
Quoted 25 lines · Kay Sievers
Quoted 10 lines · Al Viro
Quoted 6 lines · Kay Sievers
Quoted 3 lines
So we need to preserve the layout, with the easiest way probably being "add one more ktype and use kobject_init_and_add() instead of that device_create()". Sigh...
What do you mean? We just need to replace the bogus "pd->pkt_dev" with MKDEV(0, 0) and we are fine.
Userland-visible change - right now cat /sys/class/pktcdvd/pktcdvd3/dev will give you dev_t of the block device in question.
True, it's visible, but it's incorrect dead information which should be removed. Subsystem != "block is defined as S_IFCHR, and there is no such char device for pktcdvd. In fact the "dev" file of the pktcdvd class device points in many cases to a USB device, which must be fixed.
The whole pktcdvd class is not used at all, besides for exporting a few files. The "dev" file at the class device is just a bug, and the MAJOR/MINOR uevent environment variables also. I doubt that there will be any visible breakage in something that isn't already totally broken. In short, I doubt, that anything that works today will stop working if we remove the bugus "dev" file.

Quoted 8 lines

Quoted 6 lines
drivers/block/pktcdvd.c static struct kobj_type kobj_pkt_type_wqueue = { · line 305
Quoted 4 lines
This seem like the simple and correct fix for the issue with /sys/dev/ and the broken non-existing but exported pkcdvd char device:
drivers/block/pktcdvd.c +1 -1
-               pd->dev = device_create(class_pktcdvd, NULL, pd->pkt_dev, NULL,
+               pd->dev = device_create(class_pktcdvd, NULL, MKDEV(0, 0), NULL,
Al, this bug still exists, right? Any objections to remove the dead char dev_t?
Here is a patch.

I can confirm that this patch doesn't seem to cause any negative side effects on either pktsetup or pktcdvd, which are the only two user space programs I know of that care about the pktcdvd driver.

Acked-by Peter Osterlund <[email protected]>

Quoted 36 lines

Thanks,
Kay
From: Kay Sievers <[email protected]>
Subject: pktcdvd: remove broken dev_t export of class devices
The pktcdvd created class devices only export some sysfs files, but have no char dev_t registered in the driver.
At class device creation time they copy the dev_t value of the block device to the char device, wich will register a new char device in the driver core and userspace, with a conflicting dev_t value.
In many cases the class devices dev_t just points to a random USB device. This fixes the sysfs "duplicate entry" errors.
Cc [email protected]
Cc Al Viro <[email protected]>
Signed-off-by Kay Sievers <[email protected]>
---
pktcdvd.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
drivers/block/pktcdvd.c +1 -1
@@ -302,7 +302,7 @@ static struct kobj_type kobj_pkt_type_wqueue = {
 static void pkt_sysfs_dev_new(struct pktcdvd_device *pd)
 {
 	if (class_pktcdvd) {
-		pd->dev = device_create(class_pktcdvd, NULL, pd->pkt_dev, NULL,
+		pd->dev = device_create(class_pktcdvd, NULL, MKDEV(0, 0), NULL,
 					"%s", pd->name);
 		if (IS_ERR(pd->dev))
 			pd->dev = NULL;

Signature · 2 lines

Peter Osterlund - [email protected]
http://web.telia.com/~u89404340

Kay Sievers

2008-11-30 13:41 UTC #17 lore raw

In reply to Al Viro

Quoted 8 lines · Al Viro

Quoted 4 lines · Kay Sievers
Pktcdvd creates char _and_ block device nodes at the same time, while the char nodes are not allocated, but created and conflict in /sys/dev/ with properly allocated ones from other subsystems. Your patch just papers over this bug.
Where the hell is it creating any char device nodes? Show me.

Here the hell it is:
pd->dev = device_create(class_pktcdvd, NULL, pd->pkt_dev, NULL,
"%s", pd->name);

The bogus "pkt_dev", it's a char dev_t value, but because the char dev_t value is already validly used by other subsystems, it will cause the broken /sys/dev/ link. That's the real bug.

Kay