netdev - Re: [PATCH v1 1/4] ethtool: Add new hwtstamp flag

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Sep 2022 17:11:32 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Muhammad Husaini Zulkifli <muhammad.husaini.zulkifli@...el.com>
Cc:     intel-wired-lan@...osl.org, netdev@...r.kernel.org,
        davem@...emloft.net, edumazet@...gle.com, vinicius.gomes@...el.com,
        aravindhan.gunasekaran@...el.com,
        noor.azura.ahmad.tarmizi@...el.com
Subject: Re: [PATCH v1 1/4] ethtool: Add new hwtstamp flag

On Tue, 27 Sep 2022 21:06:53 +0800 Muhammad Husaini Zulkifli wrote:
> --- a/include/uapi/linux/ethtool.h
> +++ b/include/uapi/linux/ethtool.h
> @@ -675,6 +675,7 @@ enum ethtool_link_ext_substate_module {
>   * @ETH_SS_MSG_CLASSES: debug message class names
>   * @ETH_SS_WOL_MODES: wake-on-lan modes
>   * @ETH_SS_SOF_TIMESTAMPING: SOF_TIMESTAMPING_* flags
> + * @ETH_SS_HWTSTAMP_FLAG: timestamping flags
>   * @ETH_SS_TS_TX_TYPES: timestamping Tx types
>   * @ETH_SS_TS_RX_FILTERS: timestamping Rx filters
>   * @ETH_SS_UDP_TUNNEL_TYPES: UDP tunnel types
> @@ -700,6 +701,7 @@ enum ethtool_stringset {
>  	ETH_SS_MSG_CLASSES,
>  	ETH_SS_WOL_MODES,
>  	ETH_SS_SOF_TIMESTAMPING,
> +	ETH_SS_HWTSTAMP_FLAG,
>  	ETH_SS_TS_TX_TYPES,
>  	ETH_SS_TS_RX_FILTERS,
>  	ETH_SS_UDP_TUNNEL_TYPES,
> @@ -1367,6 +1369,7 @@ struct ethtool_ts_info {
>  	__u32	cmd;
>  	__u32	so_timestamping;
>  	__s32	phc_index;
> +	__u32	flag;
>  	__u32	tx_types;
>  	__u32	tx_reserved[3];
>  	__u32	rx_filters;
> diff --git a/include/uapi/linux/ethtool_netlink.h b/include/uapi/linux/ethtool_netlink.h
> index 408a664fad59..58d073b5a6d2 100644
> --- a/include/uapi/linux/ethtool_netlink.h
> +++ b/include/uapi/linux/ethtool_netlink.h
> @@ -452,6 +452,7 @@ enum {
>  	ETHTOOL_A_TSINFO_UNSPEC,
>  	ETHTOOL_A_TSINFO_HEADER,			/* nest - _A_HEADER_* */
>  	ETHTOOL_A_TSINFO_TIMESTAMPING,			/* bitset */
> +	ETHTOOL_A_TSINFO_FLAG,				/* bitset */
>  	ETHTOOL_A_TSINFO_TX_TYPES,			/* bitset */
>  	ETHTOOL_A_TSINFO_RX_FILTERS,			/* bitset */
>  	ETHTOOL_A_TSINFO_PHC_INDEX,			/* u32 */

You can't add stuff into the middle of an enum or a struct in uAPI.
What's worse for the struct ethtool_ts_info you can't actually add
anything in, period. You can reuse reserved fields but even that
requires extra legwork. If the fields were not previously validated on
input to the kernel (ie. kernel didn't check they are zero) the ioctl
path can't use them, because some application may had been passing in
garbage.