[BUG] n_gsm: possible recursive locking detected

* [BUG] n_gsm: possible recursive locking detected
@ 2019-07-17  9:40 Martin Hundebøll
  2019-07-25 11:26 ` Greg Kroah-Hartman
  0 siblings, 1 reply; 4+ messages in thread
From: Martin Hundebøll @ 2019-07-17  9:40 UTC (permalink / raw)
  To: linux-kernel; +Cc: Greg Kroah-Hartman, Jiri Slaby, Dirkjan Bussink

[-- Attachment #1: Type: text/plain, Size: 3399 bytes --]

Hi,

The GSM0710 line discipline driver triggers a lockdep warning when 
disabling the ldisc while holding a multiplexed virtual tty open:

============================================
WARNING: possible recursive locking detected
5.2.0-00114-gdab52e30156b #6 Not tainted
--------------------------------------------
cmux/263 is trying to acquire lock:
e1e23b18 (&tty->legacy_mutex){+.+.}, at: __tty_hangup.part.0+0x58/0x27c

but task is already holding lock:
d6eddf48 (&tty->legacy_mutex){+.+.}, at: tty_set_ldisc+0x3c/0x1bc

other info that might help us debug this:
  Possible unsafe locking scenario:

        CPU0
        ----
   lock(&tty->legacy_mutex);
   lock(&tty->legacy_mutex);

  *** DEADLOCK ***

  May be due to missing lock nesting notation

3 locks held by cmux/263:
  #0: d6eddf48 (&tty->legacy_mutex){+.+.}, at: tty_set_ldisc+0x3c/0x1bc
  #1: f28bead9 (&tty->ldisc_sem){++++}, at: tty_ldisc_lock+0x50/0x74
  #2: e5d20e4f (&gsm->mutex){+.+.}, at: gsm_cleanup_mux+0x9c/0x15c

stack backtrace:
CPU: 0 PID: 263 Comm: cmux Not tainted 5.2.0-00114-gdab52e30156b #6
Hardware name: Freescale i.MX6 Ultralite (Device Tree)
[<c011184c>] (unwind_backtrace) from [<c010cc74>] (show_stack+0x10/0x14)
[<c010cc74>] (show_stack) from [<c0852488>] (dump_stack+0xd4/0x108)
[<c0852488>] (dump_stack) from [<c017be90>] (__lock_acquire+0x6ec/0x1e84)
[<c017be90>] (__lock_acquire) from [<c017ddc4>] (lock_acquire+0xcc/0x204)
[<c017ddc4>] (lock_acquire) from [<c086e9d0>] (__mutex_lock+0x64/0x90c)
[<c086e9d0>] (__mutex_lock) from [<c086f294>] (mutex_lock_nested+0x1c/0x24)
[<c086f294>] (mutex_lock_nested) from [<c04c02fc>] 
(__tty_hangup.part.0+0x58/0x27c)
[<c04c02fc>] (__tty_hangup.part.0) from [<c04ce718>] 
(gsm_cleanup_mux+0xe8/0x15c)
[<c04ce718>] (gsm_cleanup_mux) from [<c04ce7d4>] (gsmld_close+0x48/0x90)
[<c04ce7d4>] (gsmld_close) from [<c04c7e24>] (tty_set_ldisc+0xb8/0x1bc)
[<c04c7e24>] (tty_set_ldisc) from [<c04c0b70>] (tty_ioctl+0x640/0xcb0)
[<c04c0b70>] (tty_ioctl) from [<c0297e68>] (do_vfs_ioctl+0x41c/0xa5c)
[<c0297e68>] (do_vfs_ioctl) from [<c02984dc>] (ksys_ioctl+0x34/0x60)
[<c02984dc>] (ksys_ioctl) from [<c0101000>] (ret_fast_syscall+0x0/0x28)
Exception stack(0xc8ce1fa8 to 0xc8ce1ff0)
1fa0:                   00438000 00000000 00000003 00005423 beb6cc04 
beb6cc04
1fc0: 00438000 00000000 00000000 00000036 00000000 00000000 00438000 
beb6ccd4
1fe0: 00438048 beb6cbfc 00427684 b6f58b88

Steps to reproduce using the attached cmux util:

root@iwg26:~# ./cmux &
[1] 254
SERIAL_PORT = /dev/ttymxc0
AT+IFC=2: Ie5   +CFUN: 1    +CPIN: READY    Call Ready  AT+IFC=2,2   OK
AT+GMM  : AT+GMM   Quectel_M95    OK
AT      : AT   OK
AT+IPR=1: AT+IPR=115200&w   OK
AT+CMUX=: AT+CMUX=0,0,5,512,10,3,30,10,2   OK
Line dicipline set

root@iwg26:~# cat /dev/gsmtty1 &
[2] 262
root@iwg26:~# kill %1
[   74.517449] ============================================
[   74.522769] WARNING: possible recursive locking detected
[   74.528094] 5.2.0-00114-gdab52e30156b #6 Not tainted
[   74.533065] --------------------------------------------
<...>

This has supposedly been fixed before in 4d9b109060f6 ("tty: Prevent 
deadlock in n_gsm driver"), but the fix was undone in be7065725590 
("TTY/n_gsm: Removing the wrong tty_unlock/lock() in gsm_dlci_release()")

-- 
Kind regards,
Martin Hundebøll
Embedded Linux Consultant

+45 61 65 54 61
martin@geanix.com

Geanix ApS
https://geanix.com
DK39600706

[-- Attachment #2: cmux.c --]
[-- Type: text/x-csrc, Size: 9492 bytes --]

/**
*	Cmux
*	Enables GSM 0710 multiplex using n_gsm
*
*	Copyright (C) 2013 - Rtone - Nicolas Le Manchet <nicolaslm@rtone.fr>
*
*	This program is free software: you can redistribute it and/or modify
*	it under the terms of the GNU General Public License as published by
*	the Free Software Foundation, either version 3 of the License, or
*	(at your option) any later version.
*
*	This program is distributed in the hope that it will be useful,
*	but WITHOUT ANY WARRANTY; without even the implied warranty of
*	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
*	GNU General Public License for more details.
*
*	You should have received a copy of the GNU General Public License
*	along with this program.  If not, see <http://www.gnu.org/licenses/>.
*
*/
#include <stdlib.h>
#include <stdio.h>
#include <stdarg.h>
#include <string.h>
#include <unistd.h>
#include <termios.h>
#include <net/if.h>
#include <linux/types.h>
#include <linux/gsmmux.h>
#include <sys/sysmacros.h>
#include <sys/ioctl.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <err.h>
#include <signal.h>

/* n_gsm ioctl */
#ifndef N_GSM0710
# define N_GSM0710	21
#endif

/* attach a line discipline ioctl */
#ifndef TIOCSETD
# define TIOCSETD	0x5423
#endif

/* serial port of the modem */
#define SERIAL_PORT	"/dev/ttymxc0"

/* line speed */
#define LINE_SPEED	B115200

/* maximum transfert unit (MTU), value in bytes */
#define MTU	512

/**
* whether or not to create virtual TTYs for the multiplex
*	0 : do not create
*	1 : create
*/
#define CREATE_NODES	0

/* number of virtual TTYs to create (most modems can handle up to 4) */
#define NUM_NODES	4

/* name of the virtual TTYs to create */
#define BASENAME_NODES	"/dev/ttyGSM"

/* name of the driver, used to get the major number */
#define DRIVER_NAME	"gsmtty"

/**
* whether or not to print debug messages to stderr
*	0 : debug off
*	1 : debug on
*/
#define DEBUG	1

/**
* whether or not to detach the program from the terminal
*	0 : do not daemonize
*	1 : daemonize
*/
#define DAEMONIZE	0

 /* size of the reception buffer which gets data from the serial line */
#define SIZE_BUF	256

/**
*	Prints debug messages to stderr if debug is wanted
*/
static void dbg(char *fmt, ...) {

	va_list args;

	if (DEBUG) {
		fflush(NULL);
		va_start(args, fmt);
		vfprintf(stderr, fmt, args);
		va_end(args);
		fprintf(stderr, "\n");
		fflush(NULL);
	}
	return;
}

/**
*	Sends an AT command to the specified line and gets its result
*	Returns  0 on success
*			-1 on failure
*/
int send_at_command(int serial_fd, char *command) {

	char buf[SIZE_BUF];
	int r;

	/* write the AT command to the serial line */
	if (write(serial_fd, command, strlen(command)) <= 0)
		err(EXIT_FAILURE, "Cannot write to %s", SERIAL_PORT);

	/* wait a bit to allow the modem to rest */
	sleep(1);

	/* read the result of the command from the modem */
	memset(buf, 0, sizeof(buf));
	r = read(serial_fd, buf, sizeof(buf));
	if (r == -1)
		err(EXIT_FAILURE, "Cannot read %s", SERIAL_PORT);

	/* if there is no result from the modem, return failure */
	if (r == 0) {
		dbg("%s\t: No response", command);
		return -1;
	}

	/* if we have a result and want debug info, strip CR & LF out from the output */
	if (DEBUG) {
		int i;
		char bufp[SIZE_BUF];
		memcpy(bufp, buf, sizeof(buf));
		for(i=0; i<strlen(bufp); i++) {
			if (bufp[i] == '\r' || bufp[i] == '\n')
				bufp[i] = ' ';
		}
		dbg("%s\t: %s", command, bufp);
	}

	/* if the output shows "OK" return success */
	if (strstr(buf, "OK\r") != NULL) {
		return 0;
	}

	return -1;		

}

/**
*	Function raised by signal catching
*/
void signal_callback_handler(int signum) {
	return;
}

/**
*	Gets the major number of the driver device
*	Returns  the major number on success
*			-1 on failure
*/
int get_major(char *driver) {

	FILE *fp;
	char *line = NULL;
	size_t len = 0;
	ssize_t read;
	char device[20];
	int major = -1;

	/* open /proc/devices file */
	if ((fp = fopen("/proc/devices", "r")) == NULL)
		err(EXIT_FAILURE, "Cannot open /proc/devices");

	/* read the file line by line */
	while ((major == -1) && (read = getline(&line, &len, fp)) != -1) {

		/* if the driver name string is found in the line, try to get the major */
		if (strstr(line, driver) != NULL) {
			if (sscanf(line,"%d %s\n", &major, device) != 2)
				major = -1;
		}

		/* free the line before getting a new one */
		if (line) {
			free(line);
			line = NULL;
		}

	}

	/* close /proc/devices file */
	fclose(fp);

	return major;
}

/**
*	Creates nodes for the virtual TTYs
*	Returns the number of nodes created
*/
int make_nodes(int major, char *basename, int number_nodes) {

	int minor, created = 0;
	dev_t device;
	char node_name[15];
	mode_t oldmask;

	/* set a new mask to get 666 mode and stores the old one */
	oldmask = umask(0);

	for (minor=1; minor<number_nodes+1; minor++) {

		/* append the minor number to the base name */
		sprintf(node_name, "%s%d", basename, minor);

		/* store a device info with major and minor */
		device = makedev(major, minor);

		/* create the actual character node */
		if (mknod(node_name, S_IFCHR | 0666, device) != 0) {
			warn("Cannot create %s", node_name);
		} else {
			created++;
			dbg("Created %s", node_name);
		}

	}

	/* revert the mask to the old one */
	umask(oldmask);

	return created;
}

/**
*	Removes previously created TTY nodes
*	Returns nothing, it doesn't really matter if it fails
*/
void remove_nodes(char *basename, int number_nodes) {

	int node;
	char node_name[15];

	for (node=1; node<number_nodes+1; node++) {

		/* append the minor number to the base name */
		sprintf(node_name, "%s%d", basename, node);

		/* unlink the actual character node */
		dbg("Removing %s", node_name);
		if (unlink(node_name) == -1)
			warn("Cannot remove %s", node_name);

	}

	return;
}

int main(void) {

	int serial_fd, major;
	struct termios tio;
	int ldisc = N_GSM0710;
	int ldisc_save;
	struct gsm_config gsm;
	char atcommand[40];
	//char disconnect[] = {0xf9, 0x03, 0xef, 0x03, 0xc3, 0x16, 0xf9};
	char disconnect[] = {0xf9, 0x03, 0xef, 0x05, 0xc3, 0x01, 0xf2, 0xf9};

	/* print global parameters */
	dbg("SERIAL_PORT = %s", SERIAL_PORT);

	/* open the serial port */
	serial_fd = open(SERIAL_PORT, O_RDWR | O_NOCTTY | O_NDELAY);
	if (serial_fd == -1)
		err(EXIT_FAILURE, "Cannot open %s", SERIAL_PORT);

	/* get the current attributes of the serial port */
	if (tcgetattr(serial_fd, &tio) == -1)
		err(EXIT_FAILURE, "Cannot get line attributes");

	/* set the new attrbiutes */
	tio.c_iflag = 0;
	tio.c_oflag = 0;
	tio.c_cflag = CS8 | CREAD | CLOCAL;
	tio.c_cflag |= CRTSCTS;
	tio.c_lflag = 0;
	tio.c_cc[VMIN] = 1;
	tio.c_cc[VTIME] = 0;

	/* write the speed of the serial line */
	if (cfsetospeed(&tio, LINE_SPEED) < 0 || cfsetispeed(&tio, LINE_SPEED) < 0)
		err(EXIT_FAILURE, "Cannot set line speed");

	/* write the attributes */
	if (tcsetattr(serial_fd, TCSANOW, &tio) == -1)
		err(EXIT_FAILURE, "Cannot set line attributes");

	/**
	*	Send AT commands to put the modem in CMUX mode.
	*	This is vendor specific and should be changed 
	*	to fit your modem needs.
	*	The following matches Quectel M95.
	*/
	if (send_at_command(serial_fd, "AT+IFC=2,2\r") == -1)
		errx(EXIT_FAILURE, "AT+IFC=2,2: bad response");	
	if (send_at_command(serial_fd, "AT+GMM\r") == -1)
		warnx("AT+GMM: bad response");
	if (send_at_command(serial_fd, "AT\r") == -1)
		warnx("AT: bad response");
	if (send_at_command(serial_fd, "AT+IPR=115200&w\r") == -1)
		errx(EXIT_FAILURE, "AT+IPR=115200&w: bad response");
	sprintf(atcommand, "AT+CMUX=0,0,5,%d,10,3,30,10,2\r", MTU);
	if (send_at_command(serial_fd, atcommand) == -1)
		errx(EXIT_FAILURE, "Cannot enable modem CMUX");

	if (ioctl(serial_fd, TIOCGETD, &ldisc_save) < 0)
		err(EXIT_FAILURE, "Cannot get current line discipline");

	/* use n_gsm line discipline */
	sleep(2);
	if (ioctl(serial_fd, TIOCSETD, &ldisc) < 0)
		err(EXIT_FAILURE, "Cannot set line dicipline. Is 'n_gsm' module registred?");

	/* get n_gsm configuration */
	if (ioctl(serial_fd, GSMIOC_GETCONF, &gsm) < 0)
		err(EXIT_FAILURE, "Cannot get GSM multiplex parameters");

	/* set and write new attributes */
	gsm.initiator = 1;
	gsm.encapsulation = 0;
	gsm.mru = MTU;
	gsm.mtu = MTU;
	gsm.t1 = 10;
	gsm.n2 = 3;
	gsm.t2 = 30;
	gsm.t3 = 10;

	if (ioctl(serial_fd, GSMIOC_SETCONF, &gsm) < 0)
		err(EXIT_FAILURE, "Cannot set GSM multiplex parameters");
	dbg("Line dicipline set");

	/* create the virtual TTYs */
	if (CREATE_NODES) {
		int created;
		if ((major = get_major(DRIVER_NAME)) < 0)
			errx(EXIT_FAILURE, "Cannot get major number");
		if ((created = make_nodes(major, BASENAME_NODES, NUM_NODES)) < NUM_NODES)
			warnx("Cannot create all nodes, only %d/%d have been created.", created, NUM_NODES);
	}

	/* detach from the terminal if needed */
	if (DAEMONIZE) {
		dbg("Going to background");
		if (daemon(0,0) != 0)
			err(EXIT_FAILURE, "Cannot daemonize");
	}

	/* wait to keep the line discipline enabled, wake it up with a signal */
	signal(SIGINT, signal_callback_handler);
	signal(SIGTERM, signal_callback_handler);
	pause();

	/* remove the created virtual TTYs */
	if (CREATE_NODES) {
		remove_nodes(BASENAME_NODES, NUM_NODES);
	}

	if (ioctl(serial_fd, TIOCSETD, &ldisc_save) < 0)
		err(EXIT_FAILURE, "Cannot set initial line dicipline");

	if (write(serial_fd, disconnect, sizeof(disconnect)) != sizeof(disconnect))
		err(EXIT_FAILURE, "Cannot disable modem CMUX");

	/* close the serial line */
	close(serial_fd);

	return EXIT_SUCCESS;
}

^ permalink raw reply	[flat|nested] 4+ messages in thread