linux-kernel - Re: [PATCH v4 2/3] vt: fix locking around vt_bind/vt

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: <20141216073758.GH2711@phenom.ffwll.local>
Date:	Tue, 16 Dec 2014 08:37:58 +0100
From:	Daniel Vetter <daniel@...ll.ch>
To:	Imre Deak <imre.deak@...el.com>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Jiri Slaby <jslaby@...e.cz>,
	Peter Hurley <peter@...leysoftware.com>,
	Daniel Vetter <daniel.vetter@...ll.ch>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 2/3] vt: fix locking around vt_bind/vt_unbind

On Tue, Dec 16, 2014 at 12:16:00AM +0200, Imre Deak wrote:
> Currently vt_bind and vt_unbind access at least the con_driver object
> and registered_con_driver array without holding the console lock. Fix
> this by locking around the whole function in each case.
> 
> Signed-off-by: Imre Deak <imre.deak@...el.com>
> Reviewed-by: Peter Hurley <peter@...leysoftware.com>

Well it's not pretty that console_lock is constantly growing and getting
pushed up the lock dep chain the past few years for various reasons, but
this here looks genuine and won't wrap new locks in console_lock.

Reviewed-by: Daniel Vetter <daniel.vetter@...ll.ch>

> ---
>  drivers/tty/vt/vt.c | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/tty/vt/vt.c b/drivers/tty/vt/vt.c
> index 9c046fb..5d36c23 100644
> --- a/drivers/tty/vt/vt.c
> +++ b/drivers/tty/vt/vt.c
> @@ -3318,11 +3318,8 @@ static int vt_bind(struct con_driver *con)
>  		if (first == 0 && last == MAX_NR_CONSOLES -1)
>  			deflt = 1;
>  
> -		if (first != -1) {
> -			console_lock();
> +		if (first != -1)
>  			do_bind_con_driver(csw, first, last, deflt);
> -			console_unlock();
> -		}
>  
>  		first = -1;
>  		last = -1;
> @@ -3362,9 +3359,7 @@ static int vt_unbind(struct con_driver *con)
>  			deflt = 1;
>  
>  		if (first != -1) {
> -			console_lock();
>  			ret = do_unbind_con_driver(csw, first, last, deflt);
> -			console_unlock();
>  			if (ret != 0)
>  				return ret;
>  		}
> @@ -3394,11 +3389,15 @@ static ssize_t store_bind(struct device *dev, struct device_attribute *attr,
>  	struct con_driver *con = dev_get_drvdata(dev);
>  	int bind = simple_strtoul(buf, NULL, 0);
>  
> +	console_lock();
> +
>  	if (bind)
>  		vt_bind(con);
>  	else
>  		vt_unbind(con);
>  
> +	console_unlock();
> +
>  	return count;
>  }
>  
> -- 
> 1.8.4
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/