Re: [PATCH] check shm mount succeeded in shmem_file_setup

Re: [PATCH] check shm mount succeeded in shmem_file_setup

[Rusty Russell]

In message <200207190354.WAA05241@ccure.karaya.com> you write:
> The kern_mount(&tmpfs_fs_type) in init_shmem_fs can fail, leaving shm_mnt
> NULL.  A subsequent shmget will enter shmem_file_setup, which will blindly
> dereference shm_mnt.  EIO was my best guess as to the appropriate errno.

I think the bug is checking the return value at all.  This code cannot
be a module (at least without significant furthur work), despite the
fact that someone nicely wrote an exitfunction for it.

And if the initialization fails at boot, we're screwed anyway.

> --- orig/mm/shmem.c     Mon Feb 25 12:50:45 2002
> +++ um/mm/shmem.c       Thu Jul 18 22:16:11 2002
> @@ -1455,6 +1455,9 @@
>         if (!vm_enough_memory((size) >> PAGE_CACHE_SHIFT))
>                 return ERR_PTR(-ENOMEM);
>  
> +       if(shm_mnt == NULL)
> +               return ERR_PTR(-EIO);
> +
>         this.name = name;
>         this.len = strlen(name);
>         this.hash = 0; /* will go */

Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] check shm mount succeeded in shmem_file_setup

[Jeff Dike]

rusty@rustcorp.com.au said:
> And if the initialization fails at boot, we're screwed anyway. 

Why?  If it fails, it still boots fine until something tries using shared
memory.  With UML and my Debian fs, that's Apache, which is the last thing
before the gettys run.

				Jeff

Re: [PATCH] check shm mount succeeded in shmem_file_setup

[Rusty Russell]

In message <200207191502.KAA02022@ccure.karaya.com> you write:
> rusty@rustcorp.com.au said:
> > And if the initialization fails at boot, we're screwed anyway. 
> 
> Why?  If it fails, it still boots fine until something tries using shared
> memory.  With UML and my Debian fs, that's Apache, which is the last thing
> before the gettys run.

Same argument applies to lots of subsystems, but I'd suggest a policy:
we should be failing the boot rather than coming partially up and
trying to deal with failures that shouldn't happen.

Unfortunately, we don't check init returns at boot, because we
*expect* device driver initialization to fail for builtin device
drivers who have found no device.  It'd be nice to standardize on
-ENODEV for these failures, so we *could* handle these failures
easily, and discourage the current sloppiness.

Cheers,
Rusty.
--
  Anyone who quotes me in their sig is an idiot. -- Rusty Russell.

Re: [PATCH] check shm mount succeeded in shmem_file_setup

[Jeff Dike]

rusty@rustcorp.com.au said:
> Same argument applies to lots of subsystems, but I'd suggest a policy:
> we should be failing the boot rather than coming partially up and
> trying to deal with failures that shouldn't happen. 

OK, I can buy that.

			Jeff