[PATCH] Fix crash on termination before udev queue initialization

[PATCH] Fix crash on termination before udev queue initialization

[Nix]

On 16 May 2011, Kay Sievers stated:

> Commited a fix to git, that moves the creation of the queue file above
> the daemonize. Hopefully that's the race you are seeing.

Hm, not done more than compile it yet, but there's definitely still
something wrong here: it coredumps if called with invalid arguments.

mutilate:/lib/udev/rules.d# gdb --args /sbin/udevd --no-daemon
[...]
Program received signal SIGSEGV, Segmentation fault.
0x0000000000418a0c in udev_queue_export_cleanup ()
(gdb) bt
#0  udev_queue_export_cleanup (udev_queue_export=0x0) at libudev/libudev-queue-private.c:107
#1  0x00007fffffffc130 in ?? ()
#2  0x00007fffffffc070 in ?? ()
#3  0x0000000000000000 in ?? ()

Fixed by initializing the queue variable to NULL at startup, and making
udev_queue_export_cleanup() just return if called with NULL, like
free().
---
 libudev/libudev-queue-private.c |    3 +++
 udev/udevd.c                    |    2 +-
 2 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/libudev/libudev-queue-private.c b/libudev/libudev-queue-private.c
index 0dcf9b0..747d555 100644
--- a/libudev/libudev-queue-private.c
+++ b/libudev/libudev-queue-private.c
@@ -104,6 +104,9 @@ void udev_queue_export_cleanup(struct udev_queue_export *udev_queue_export)
 {
 	char filename[UTIL_PATH_SIZE];
 
+	if (udev_queue_export = NULL)
+		return;
+
 	util_strscpyl(filename, sizeof(filename), udev_get_run_path(udev_queue_export->udev), "/queue.tmp", NULL);
 	unlink(filename);
 
diff --git a/udev/udevd.c b/udev/udevd.c
index e7384e1..937717a 100644
--- a/udev/udevd.c
+++ b/udev/udevd.c
@@ -69,7 +69,7 @@ static void log_fn(struct udev *udev, int priority,
 }
 
 static struct udev_rules *rules;
-static struct udev_queue_export *udev_queue_export;
+static struct udev_queue_export *udev_queue_export = NULL;
 static struct udev_ctrl *udev_ctrl;
 static struct udev_monitor *monitor;
 static int worker_watch[2] = { -1, -1 };
-- 
1.7.5.1.133.geb78b

Re: [PATCH] Fix crash on termination before udev queue initialization

[Kay Sievers]

On Mon, May 16, 2011 at 22:40, Nix <nix@esperi.org.uk> wrote:
> On 16 May 2011, Kay Sievers stated:
>
>> Commited a fix to git, that moves the creation of the queue file above
>> the daemonize. Hopefully that's the race you are seeing.
>
> Hm, not done more than compile it yet, but there's definitely still
> something wrong here: it coredumps if called with invalid arguments.

> --- a/libudev/libudev-queue-private.c
> +++ b/libudev/libudev-queue-private.c
> @@ -104,6 +104,9 @@ void udev_queue_export_cleanup(struct udev_queue_export *udev_queue_export)
>  {
>        char filename[UTIL_PATH_SIZE];
>
> +       if (udev_queue_export = NULL)
> +               return;
> +

Committed.

> --- a/udev/udevd.c
> +++ b/udev/udevd.c
> @@ -69,7 +69,7 @@ static void log_fn(struct udev *udev, int priority,
>  }
>
>  static struct udev_rules *rules;
> -static struct udev_queue_export *udev_queue_export;
> +static struct udev_queue_export *udev_queue_export = NULL;

That's not needed for static vars.

Thanks,
Kay

Re: [PATCH] Fix crash on termination before udev queue initialization

[Nix]

On 16 May 2011, Kay Sievers stated:
>> --- a/udev/udevd.c
>> +++ b/udev/udevd.c
>> @@ -69,7 +69,7 @@ static void log_fn(struct udev *udev, int priority,
>>  }
>>
>>  static struct udev_rules *rules;
>> -static struct udev_queue_export *udev_queue_export;
>> +static struct udev_queue_export *udev_queue_export = NULL;
>
> That's not needed for static vars.

Oh yes. *slaps self*

Obviously I am too tired to be working on Hello World, let alone stuff
running as root...

btw, that patch might have worked. I had a trouble-free boot without
pausing this time... but of course this is an intermittent bug so it
is hard to be sure.

I'll yell if it goes wrong again.

-- 
NULL && (void)