linux-kernel - Re: [RFC 1/1] tty: n_gsm: improve standard compliance and feature completeness

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-next>] [day] [month] [year] [list]

Date:   Wed, 2 Mar 2022 14:47:20 +0000
From:   "Starke, Daniel" <daniel.starke@...mens.com>
To:     Greg KH <gregkh@...uxfoundation.org>
CC:     "linux-serial@...r.kernel.org" <linux-serial@...r.kernel.org>,
        "jirislaby@...nel.org" <jirislaby@...nel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [RFC 1/1] tty: n_gsm: improve standard compliance and feature
 completeness

Thank you for this quick response.

> I'm sorry, but there is nothing we can do with such a large patch here at
> all.
> 
> Please break this up into "one logical change per patch" and we will be
> glad to review it.

You are absolutely right but this is still work in progress. The attached
patch was only for reference for all those who would like to look at the
changes in detail or try out the proposed work. This was not a request for
code review yet. Also, it seems that the patch was too large for the
linux-serial mailing list so I have uploaded it here for reference:
https://github.com/siemens/linux/tree/dstarke-siemens/n_gsm_rfc

> Put your "fixes" at the beginning of the patch series, so they can be
> properly backported to older kernels as needed. Don't mix up new features
> with fixes as that means that the fixes can never be backported.

All fixes that have no dependency on the upcoming features have been
submitted by me already. I will double check the remaining fixes but I
could not find a way to separate those from the new features so far.

> One note, please drop the useless MODULE_VERSION() that never belongs in
> an in-kernel driver as it makes no sense (the version is the kernel
> version).

This helped us during our internal development and test procedure. But I
will remove it before submitting the actual patches.

> Same for the license "boiler-plate" text, that should not be added back,
> we removed it for a reason.

Will be removed.

> Also, no module parameters should be added. Especially pointless ones
> like debugging flags, use the built-in kernel debugging options that the
> whole rest of the kernel uses. There is no need for a special case for
> just one tiny line discipline module :)

The module parameter "debug" is not new. It is included in all kernel
version. And I did not plan to remove it. Especially, since this protocol
is complex and hard to debug with the provided kernel debugging options
only.
My addition was the parameter "strict" to verify standard conformance of
connected devices with the underlying protocol. Otherwise, the protocol
gracefully handles invalid values. Would you suggest to make this parameter
an ioctl instead?

With best regards,
Daniel Starke