Skip to content

Commit

Permalink
tty: n_gsm: fix decoupled mux resource
Browse files Browse the repository at this point in the history
commit 1ec92e9 upstream.

The active mux instances are managed in the gsm_mux array and via mux_get()
and mux_put() functions separately. This gives a very loose coupling
between the actual instance and the gsm_mux array which manages it. It also
results in unnecessary lockings which makes it prone to failures. And it
creates a race condition if more than the maximum number of mux instances
are requested while the user changes the parameters of an active instance.
The user may loose ownership of the current mux instance in this case.
Fix this by moving the gsm_mux array handling to the mux allocation and
deallocation functions.

Fixes: e1eaea4 ("tty: n_gsm line discipline")
Cc: stable@vger.kernel.org
Signed-off-by: Daniel Starke <daniel.starke@siemens.com>
Link: https://lore.kernel.org/r/20220414094225.4527-3-daniel.starke@siemens.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
dstarke-siemens authored and gregkh committed May 9, 2022
1 parent 7f71387 commit 8b19dfb
Showing 1 changed file with 38 additions and 25 deletions.
63 changes: 38 additions & 25 deletions drivers/tty/n_gsm.c
Expand Up @@ -2078,18 +2078,6 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc)
/* Finish outstanding timers, making sure they are done */
del_timer_sync(&gsm->t2_timer);

spin_lock(&gsm_mux_lock);
for (i = 0; i < MAX_MUX; i++) {
if (gsm_mux[i] == gsm) {
gsm_mux[i] = NULL;
break;
}
}
spin_unlock(&gsm_mux_lock);
/* open failed before registering => nothing to do */
if (i == MAX_MUX)
return;

/* Free up any link layer users */
for (i = 0; i < NUM_DLCI; i++)
if (gsm->dlci[i])
Expand All @@ -2113,7 +2101,6 @@ static void gsm_cleanup_mux(struct gsm_mux *gsm, bool disc)
static int gsm_activate_mux(struct gsm_mux *gsm)
{
struct gsm_dlci *dlci;
int i = 0;

timer_setup(&gsm->t2_timer, gsm_control_retransmit, 0);
init_waitqueue_head(&gsm->event);
Expand All @@ -2125,18 +2112,6 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
else
gsm->receive = gsm1_receive;

spin_lock(&gsm_mux_lock);
for (i = 0; i < MAX_MUX; i++) {
if (gsm_mux[i] == NULL) {
gsm->num = i;
gsm_mux[i] = gsm;
break;
}
}
spin_unlock(&gsm_mux_lock);
if (i == MAX_MUX)
return -EBUSY;

dlci = gsm_dlci_alloc(gsm, 0);
if (dlci == NULL)
return -ENOMEM;
Expand All @@ -2152,6 +2127,15 @@ static int gsm_activate_mux(struct gsm_mux *gsm)
*/
static void gsm_free_mux(struct gsm_mux *gsm)
{
int i;

for (i = 0; i < MAX_MUX; i++) {
if (gsm == gsm_mux[i]) {
gsm_mux[i] = NULL;
break;
}
}
mutex_destroy(&gsm->mutex);
kfree(gsm->txframe);
kfree(gsm->buf);
kfree(gsm);
Expand All @@ -2171,12 +2155,20 @@ static void gsm_free_muxr(struct kref *ref)

static inline void mux_get(struct gsm_mux *gsm)
{
unsigned long flags;

spin_lock_irqsave(&gsm_mux_lock, flags);
kref_get(&gsm->ref);
spin_unlock_irqrestore(&gsm_mux_lock, flags);
}

static inline void mux_put(struct gsm_mux *gsm)
{
unsigned long flags;

spin_lock_irqsave(&gsm_mux_lock, flags);
kref_put(&gsm->ref, gsm_free_muxr);
spin_unlock_irqrestore(&gsm_mux_lock, flags);
}

static inline unsigned int mux_num_to_base(struct gsm_mux *gsm)
Expand All @@ -2197,6 +2189,7 @@ static inline unsigned int mux_line_to_num(unsigned int line)

static struct gsm_mux *gsm_alloc_mux(void)
{
int i;
struct gsm_mux *gsm = kzalloc(sizeof(struct gsm_mux), GFP_KERNEL);
if (gsm == NULL)
return NULL;
Expand Down Expand Up @@ -2226,6 +2219,26 @@ static struct gsm_mux *gsm_alloc_mux(void)
gsm->mtu = 64;
gsm->dead = true; /* Avoid early tty opens */

/* Store the instance to the mux array or abort if no space is
* available.
*/
spin_lock(&gsm_mux_lock);
for (i = 0; i < MAX_MUX; i++) {
if (!gsm_mux[i]) {
gsm_mux[i] = gsm;
gsm->num = i;
break;
}
}
spin_unlock(&gsm_mux_lock);
if (i == MAX_MUX) {
mutex_destroy(&gsm->mutex);
kfree(gsm->txframe);
kfree(gsm->buf);
kfree(gsm);
return NULL;
}

return gsm;
}

Expand Down

0 comments on commit 8b19dfb

Please sign in to comment.