Chuck Ebbert wrote:
> In-Reply-To: <[EMAIL PROTECTED]>
>
> On Sun, 03 Dec 2006 02:55:04 +0300, Michael Tokarev wrote:
>
>> When /sbin/hotplug is present in initramfs, and it's a shell
>> script, kernel OOPSes on every hotplug invocation.
>
> Does this mean that if it's _not_ a shell script everything
> is fine?
It means that if there's no pipe() call, everything is fine.
That is, for a shell script it's just more likely to contain
some pipelines.
[]
>> <0>EFLAGS: 00010282 (2.6.19-c3 #2.6.19.0)
[]
>> <4> <1>BUG: unable to handle kernel NULL pointer dereference at virtual
>> address 00000014
>> (note also the formatting is a bit wrong here -- that <4> prefix in front of
>> <1>BUG..)
>
> Yeah, that's from the previous oops. I don't know why bust_spinlocks() leaves
> that hanging space, but it's been that way for a long time.
So can this be fixed, too? ;)
> BTW did you hand-edit the kernel .version file? "(2.6.19-c3 #2.6.19.0)"
> should not print unless you put that 2.6.19.0 there yourself.
Yes I did. For an rpm- or deb-based kernel, the "build number" is irrelevant
(it's always 1, since both rpm and deb rebuilds everything from scratch).
This #2.6.19.0 comes from the package revision number - I just do
echo $revision > .version
and remove the part from Makefile which updates the file on every (re)build.
But this isn't relevant for the discussion.
>> Are pipes disallowed in hotplug fired off initramfs?
>
> AFAICT the pipe filesystem isn't registered yet, so probably not.
>
>> (Even if they are, kernel probably still should not OOPS like that ;)
>
> Either (a) pipefs should be registered early, or (b) the call should just
> fail instead of oopsing. Untested patch for (b) below, can you test it?
> (I'm not messing with the init sequence.)
>
>> The error seems to be harmful however. That is, hotplug script does not
>> run, but the kernel continues running.
>
> You mean harmless, right?
Yup. Oh those foreign languages... :)
> Signed-off-by: Chuck Ebbert <[EMAIL PROTECTED]>
>
> --- 2.6.19.0.2-32.orig/fs/pipe.c
> +++ 2.6.19.0.2-32/fs/pipe.c
Another funny revision number? ;)
> @@ -839,9 +839,11 @@ static struct dentry_operations pipefs_d
>
> static struct inode * get_pipe_inode(void)
> {
> - struct inode *inode = new_inode(pipe_mnt->mnt_sb);
> + struct inode *inode = NULL;
> struct pipe_inode_info *pipe;
>
> + if (pipe_mnt)
> + inode = new_inode(pipe_mnt->mnt_sb);
> if (!inode)
> goto fail_inode;
Well, that fixes the OOPs, but I'd do it a in bit more readable way,
and eliminating the gotos too (this is a stylistic change - I see alot
of gotos in kernel for error returns, but here, in such a small function,
gotos looks somewhat funny, esp. when the only statement after the label
is 'return NULL'):
--- linux-2.6.19/fs/pipe.c.orig 2006-11-30 00:57:37.000000000 +0300
+++ linux-2.6.19/fs/pipe.c 2006-12-03 14:27:06.000000000 +0300
@@ -839,15 +839,20 @@ static struct dentry_operations pipefs_d
static struct inode * get_pipe_inode(void)
{
- struct inode *inode = new_inode(pipe_mnt->mnt_sb);
+ struct inode *inode;
struct pipe_inode_info *pipe;
+ if (!pipe_mnt) /* if pipefs isn't mounted (yet) */
+ return NULL;
+ inode = new_inode(pipe_mnt->mnt_sb);
if (!inode)
- goto fail_inode;
+ return NULL;
pipe = alloc_pipe_info(inode);
- if (!pipe)
- goto fail_iput;
+ if (!pipe) {
+ iput(inode);
+ return NULL;
+ }
inode->i_pipe = pipe;
pipe->readers = pipe->writers = 1;
@@ -866,12 +871,6 @@ static struct inode * get_pipe_inode(voi
inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME;
return inode;
-
-fail_iput:
- iput(inode);
-
-fail_inode:
- return NULL;
}
struct file *create_write_pipe(void)
Or like this (without messing up with goto):
--- linux-2.6.19/fs/pipe.c.orig 2006-11-30 00:57:37.000000000 +0300
+++ linux-2.6.19/fs/pipe.c 2006-12-03 14:31:19.000000000 +0300
@@ -839,9 +839,12 @@ static struct dentry_operations pipefs_d
static struct inode * get_pipe_inode(void)
{
- struct inode *inode = new_inode(pipe_mnt->mnt_sb);
+ struct inode *inode;
struct pipe_inode_info *pipe;
+ if (!pipe_mnt) /* in case pipefs isn't mounted (yet) */
+ goto fail_inode;
+ inode = new_inode(pipe_mnt->mnt_sb);
if (!inode)
goto fail_inode;
In either case, this change not fixes the problem. As I said, the OOPs is
harmless (yes, not harmful :), so the end effect is the same, just instead
of the OOPs, now the bad stuff is coming from shell complaining about failing
to create pipe.
/mjt
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
