linux-kernel - Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Use (pr|netdev)

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <201006160023.04296.sven.eckelmann@gmx.de>
Date:	Wed, 16 Jun 2010 00:23:02 +0200
From:	Sven Eckelmann <sven.eckelmann@....de>
To:	b.a.t.m.a.n@...ts.open-mesh.org
Cc:	Joe Perches <joe@...ches.com>,
	Marek Lindner <lindner_marek@...oo.de>,
	Simon Wunderlich <siwu@....tu-chemnitz.de>,
	Andrew Lunn <andrew@...n.ch>,
	"Greg Kroah-Hartman" <gregkh@...e.de>,
	devel <devel@...verdev.osuosl.org>,
	b.a.t.m.a.n@...ts.open-mesh.net, linux-kernel@...r.kernel.org
Subject: Re: [B.A.T.M.A.N.] [PATCH] drivers/staging/batman-adv: Use (pr|netdev)_<level> macro helpers

Joe Perches wrote:
> Compile tested only
> 
> Add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
> Remove "batman-adv:" from format strings
> Use pr_<level>
> Use netdev_<level>
> 
> Signed-off-by: Joe Perches <joe@...ches.com>
> ---

Found some time to play a little bit around with your patch and noticed that
it crashes the kernel. I did my tests using following "interesting" part:

[....]
> @@ -152,18 +155,18 @@ static ssize_t store_vis_mode(struct kobject *kobj,
> struct attribute *attr, if (buff[count - 1] == '\n')
>  			buff[count - 1] = '\0';
> 
> -		printk(KERN_INFO "batman-adv:Invalid parameter for 'vis mode' setting on mesh %s received: %s\n",
> -		       net_dev->name, buff);
> +		netdev_info(net_dev, "Invalid parameter for 'vis mode' setting on mesh received: %s\n",
> +			    buff);
>  		return -EINVAL;
>  	}
> 
>  	if (atomic_read(&bat_priv->vis_mode) == vis_mode_tmp)
>  		return count;
> 
> -	printk(KERN_INFO "batman-adv:Changing vis mode from: %s to: %s on mesh: %s\n",
> -	       atomic_read(&bat_priv->vis_mode) == VIS_TYPE_CLIENT_UPDATE ?
> -	       "client" : "server", vis_mode_tmp == VIS_TYPE_CLIENT_UPDATE ?
> -	       "client" : "server", net_dev->name);
> +	netdev_info(net_dev, "Changing vis mode from: %s to: %s on mesh\n",
> +		    atomic_read(&bat_priv->vis_mode) == VIS_TYPE_CLIENT_UPDATE ?
> +		    "client" : "server",
> +		    vis_mode_tmp == VIS_TYPE_CLIENT_UPDATE ? "client" : "server");
> 
>  	atomic_set(&bat_priv->vis_mode, (unsigned)vis_mode_tmp);
>  	return count;
[...]

I compiled the module and loaded it using `insmod batman-adv.ko`. This will
create some files in /sys. Just changed the vis mode to server using:

echo 0 > /sys/class/net/bat0/mesh/vis_mode

And then it will crash at that netdev_info call.

The problem seems to be that dev_printk is used by netdev_printk (which is
used by netdev_info). netdev_printk will add (netdev)->dev.parent as second
parameter of dev_printk (and parent is NULL in our case). This macro will now
call dev_driver_string with NULL as parameter and just dereference this null pointer.

Maybe it is related to something else, but at least I think that this could be
the cause of the crash.

Best regards,
	Sven

Download attachment "signature.asc " of type "application/pgp-signature" (837 bytes)