linux-kernel - Re: [PATCH v5 09/10] tty: n_gsm: expose configuration and statistics via proc fs

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]

Date:   Tue, 30 May 2023 12:23:46 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "D. Starke" <daniel.starke@...mens.com>
Cc:     linux-serial@...r.kernel.org, jirislaby@...nel.org,
        ilpo.jarvinen@...ux.intel.com, felix-haase@...mens.com,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH v5 09/10] tty: n_gsm: expose configuration and statistics
 via proc fs

On Wed, May 17, 2023 at 05:57:03PM +0200, D. Starke wrote:
> From: Daniel Starke <daniel.starke@...mens.com>
> 
> The n_gsm mux collects various statistics about the mux and its channels.
> These are currently not exposed to the user. There exists already a proc fs
> path for tty ldiscs (/proc/tty/ldisc).
> 
> Extend this path by an 'n_gsm' node and create a proc file for each mux
> instance if active. The file exposes protocol statistics and channel states
> and configuration to the user. Mutex based locks are introduced to avoid
> inconsistent states.
> 
> The following shows an example output:
>  tty:ttyS1 flags:
>  initiator:1 mode:1 mru:64 mtu:64 t1:10 t2:34 t3:10 n2:3 k:2 wc:0 ka:100
>  bad_fcs:0 malformed:0 io_error:0 open_error:0 bad_size:0 unsupported:0
> 
>  dlci:0 state:OPEN cl:2 prio:0 i:UIH k:2 mtu:64 tx:35 rx:35
> 
> Description:
> tty:         The underlying device used by this mux.
> flags:       Tty flags relevant to the mux protocol.
> initiator:   0 for responder, 1 for initiator.
> mode:        0 for basic option mode, 1 for advanced option mode
> mru:         Maximum receive unit size.
> mtu:         Maximum transmission unit size.
> t1:          Acknowledgment timer.
> t2:          Response timer for multiplexer control channel.
> t3:          Response timer for wake-up procedure.
> n2:          Maximum number of retransmissions.
> k:           Window size.
> wc:          Wait for configuration before starting parameter negotiation?
> ka:          Control channel keep-alive timer (0 if disabled).
> bad_fcs:     Number of bad FCS.
> malformed:   Number of malformed frames.
> io_error:    Number of I/O errors on the underlying tty.
> open_error:  Number of failed DLCI open attempts.
> bad_size:    Number of n_gsm frames with bad size.
> unsupported: Number of malformed control frames.
> dlci:        Related channel number.
> state:       Current channel state. Possible values are CLOSED,
>              WAITING_CONFIG, CONFIGURE, OPENING, OPEN and CLOSING.
> cl:          Convergence layer type.
> prio:        Priority.
> i:           Framing. Possible values are UI and UIH.
> k:           Window size.
> mtu:         Maximum transmission unit size.
> tx:          Transmitted payload size in bytes (incl. convergence layer
>              type header).
> rx:          Received payload size in bytes (incl. convergence layer type
>              header).
> 
> All timers are in 1/100th of a second units.

Please no, procfs is NOT for driver/device statistics like this, that's
what sysfs is for if you really need/want it.

What userspace tool is going to read/parse this thing?  Where does it
live?

And what about the security issues involved with all of this new data
that you are now exposing to all users?  Has it been audited to verify
that it is safe to do so?

thanks,

greg k-h