linux-kernel - Re: [PATCH 9/9] TTY: call tty_port

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: <50A8E521.10509@imap.cc>
Date:	Sun, 18 Nov 2012 14:39:45 +0100
From:	Tilman Schmidt <tilman@...p.cc>
To:	Jiri Slaby <jslaby@...e.cz>
CC:	gregkh@...uxfoundation.org, alan@...ux.intel.com,
	linux-kernel@...r.kernel.org, jirislaby@...il.com
Subject: Re: [PATCH 9/9] TTY: call tty_port_destroy in the rest of drivers

Hi Jiri,

Two remarks wrt the Gigaset driver.

Am 15.11.2012 09:49, schrieb Jiri Slaby:
> diff --git a/drivers/isdn/gigaset/common.c b/drivers/isdn/gigaset/common.c
> index 30a6b17..bc9d89a 100644
> --- a/drivers/isdn/gigaset/common.c
> +++ b/drivers/isdn/gigaset/common.c
> @@ -518,6 +518,7 @@ f_bcs:	gig_dbg(DEBUG_INIT, "freeing bcs[]");
>  	kfree(cs->bcs);
>  f_cs:	gig_dbg(DEBUG_INIT, "freeing cs");
>  	mutex_unlock(&cs->mutex);
> +	tty_port_destroy(&cs->port);
>  	free_cs(cs);
>  }
>  EXPORT_SYMBOL_GPL(gigaset_freecs);

It is not ok to call tty_port_destroy() unconditionally here.
gigaset_freecs() may be called from gigaset_initcs() before
the tty_port_init(&cs->port) call if a memory allocation fails.
This is best fixed by moving that call to case 1 of the preceding
switch statement because cs_init >= 1 covers exactly the cases
where the tty_port_init(&cs->port) call has already been passed.

> @@ -751,14 +752,14 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  	gig_dbg(DEBUG_INIT, "setting up iif");
>  	if (gigaset_isdn_regdev(cs, modulename) < 0) {
>  		pr_err("error registering ISDN device\n");
> -		goto error;
> +		goto error_port;
>  	}
>  
>  	make_valid(cs, VALID_ID);
>  	++cs->cs_init;
>  	gig_dbg(DEBUG_INIT, "setting up hw");
>  	if (cs->ops->initcshw(cs) < 0)
> -		goto error;
> +		goto error_port;
>  
>  	++cs->cs_init;
>  
> @@ -773,7 +774,7 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  		gig_dbg(DEBUG_INIT, "setting up bcs[%d]", i);
>  		if (gigaset_initbcs(cs->bcs + i, cs, i) < 0) {
>  			pr_err("could not allocate channel %d data\n", i);
> -			goto error;
> +			goto error_port;
>  		}
>  	}
>  
> @@ -786,7 +787,8 @@ struct cardstate *gigaset_initcs(struct gigaset_driver *drv, int channels,
>  
>  	gig_dbg(DEBUG_INIT, "cs initialized");
>  	return cs;
> -
> +error_port:
> +	tty_port_destroy(&cs->port);
>  error:
>  	gig_dbg(DEBUG_INIT, "failed");
>  	gigaset_freecs(cs);

You have already added a tty_port_destroy() call to gigaset_freecs(cs)
above. Adding another one here will lead to the port being destroyed
twice in this code path.

Thanks,
Tilman

-- 
Tilman Schmidt                    E-Mail: tilman@...p.cc
Bonn, Germany
Diese Nachricht besteht zu 100% aus wiederverwerteten Bits.
Ungeöffnet mindestens haltbar bis: (siehe Rückseite)


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