[PATCH] automount: check for recursive automount

[PATCH] automount: check for recursive automount

[Sascha Hauer]

automount_mount calls run_command which may trigger an automount
again. This results in an endless loop. A simple way to trigger
this is:

mkdir /x; automount /x false; cd /x; something

Use a static variable to detect if we are currently in automount_mount()
and bail out if we are.

Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de>
---
 fs/fs.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/fs/fs.c b/fs/fs.c
index b0ac918..dd410b7 100644
--- a/fs/fs.c
+++ b/fs/fs.c
@@ -409,6 +409,12 @@ static void automount_mount(const char *path, int instat)
 {
 	struct automount *am;
 	int ret;
+	static int in_automount;
+
+	if (in_automount)
+		return;
+
+	in_automount++;
 
 	list_for_each_entry(am, &automount_list, list) {
 		int len_path = strlen(path);
@@ -444,8 +450,10 @@ static void automount_mount(const char *path, int instat)
 		else
 			automount_remove(am->path);
 
-		return;
+		break;
 	}
+
+	in_automount--;
 }
 
 BAREBOX_MAGICVAR(automount_path, "mountpath passed to automount scripts");
-- 
2.0.0


_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] automount: check for recursive automount

[Rolf Evers-Fischer]

Dear Uwe and Alessandro,
thank you for your quick replies and the interesting informations that you 
shared. I didn't know these details until now.

Best regards,
 Rolf

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] automount: check for recursive automount

[Uwe Kleine-König]

On Thu, Jul 03, 2014 at 11:00:45AM +0200, Alessandro Rubini wrote:
> > static variables are initialized to 0 (for pointers: NULL) by the
> > compiler.
> 
> Well, not really. The compiler just places them in the .bss area.  It
> is barebox, it is run-time initialization, that zeroes bss.
> 
> So it is barebox, not the compiler.
Yeah, in general it's even worse because there might be platforms where
NULL isn't represented by a literal 0. (Not sure if something like this
exists, but it's allowed by the standard.)

C99 tells us:

If an object that has automatic storage duration is not initialized explicitly,
its value is indeterminate. If an object that has static storage duration is
not initialized explicitly, then:
- if it has pointer type, it is initialized to a null pointer;
- if it has arithmetic type, it is initialized to (positive or unsigned) zero;
- if it is an aggregate, every member is initialized (recursively) according to
  these rules;
- if it is a union, the first named member is initialized (recursively)
  according to these rules.

I assume it's included in earlier standards, too, but I don't have these
handy.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] automount: check for recursive automount

[Alessandro Rubini]

> static variables are initialized to 0 (for pointers: NULL) by the
> compiler.

Well, not really. The compiler just places them in the .bss area.  It
is barebox, it is run-time initialization, that zeroes bss.

So it is barebox, not the compiler.

Uwe, I know you know this, but I'd better make it clear for
everybody. I've already seen a "specialist" writing an IPL where he
explicitly sets static variables to 0 one by one in different places
because he didn't clear bss in his assembly.

/alessandro

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] automount: check for recursive automount

[Uwe Kleine-König]

On Thu, Jul 03, 2014 at 10:34:37AM +0200, Rolf Evers-Fischer wrote:
> Hi Sascha,
> 
> Sascha Hauer <s.hauer@...> wrote:
> > +	static int in_automount;
> > +
> > +	if (in_automount)
> > +		return;
> 
> It seems that we are using the 'in_automount' without initialization. 
> Shouldn't we initialize the static 'in_automount' with 0? Or can we rely 
> on the compiler, that all statics will be initialized with zeros?
static variables are initialized to 0 (for pointers: NULL) by the
compiler.

Best regards
Uwe

-- 
Pengutronix e.K.                           | Uwe Kleine-König            |
Industrial Linux Solutions                 | http://www.pengutronix.de/  |

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox

Re: [PATCH] automount: check for recursive automount

[Rolf Evers-Fischer]

Hi Sascha,

Sascha Hauer <s.hauer@...> wrote:
> +	static int in_automount;
> +
> +	if (in_automount)
> +		return;

It seems that we are using the 'in_automount' without initialization. 
Shouldn't we initialize the static 'in_automount' with 0? Or can we rely 
on the compiler, that all statics will be initialized with zeros?

Best regards,
 Rolf

_______________________________________________
barebox mailing list
barebox@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/barebox