linux-kernel - RE: [PATCH 5/9] tty: n_gsm: add open_error counter to gsm

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:   Thu, 6 Apr 2023 05:42:50 +0000
From:   "Starke, Daniel" <daniel.starke@...mens.com>
To:     Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
CC:     linux-serial <linux-serial@...r.kernel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Jiri Slaby <jirislaby@...nel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 5/9] tty: n_gsm: add open_error counter to gsm_mux

> > @@ -1767,6 +1775,7 @@ static void gsm_control_negotiation(struct gsm_mux *gsm, unsigned int cr,
> >  	} else {
> >  		if (debug & DBG_ERRORS)
> >  			pr_info("%s PN in invalid state\n", __func__);
> > +		gsm->open_error++;
> >  	}
> 
> I'd use the "rollback" pattern here for all these and goto open_failed;
> + do the open_error increment there only once.

True, that would be a more elegant way to handle this. However, it does
more than just adding this counter. Therefore, I would prefer to do this
in a later cleanup.

...
> In general, the changelog could be more verbose about state machine 
> states, message names which imply that the error is happening during 
> "opening" phase/state.

I will add a more detailed description.

Best regards,
Daniel Starke