Re: [PATCH net-next] genetlink: start to validate reserved header bytes
Hi Jakub,
On Wed, Aug 24, 2022 at 05:18:30PM -0700, Jakub Kicinski wrote:
> diff --git a/drivers/net/wireguard/netlink.c b/drivers/net/wireguard/netlink.c
> index d0f3b6d7f408..0c0644e762e5 100644
> --- a/drivers/net/wireguard/netlink.c
> +++ b/drivers/net/wireguard/netlink.c
> @@ -621,6 +621,7 @@ static const struct genl_ops genl_ops[] = {
> static struct genl_family genl_family __ro_after_init = {
> .ops = genl_ops,
> .n_ops = ARRAY_SIZE(genl_ops),
> + .resv_start_op = WG_CMD_SET_DEVICE + 1,
> .name = WG_GENL_NAME,
> .version = WG_GENL_VERSION,
> .maxattr = WGDEVICE_A_MAX,
FWIW, I wouldn't object to just leaving this at zero. I don't know of
any wireguard userspaces doing anything with the reserved header field.
Jason
Re: [PATCH net-next] genetlink: start to validate reserved header bytes
On Wed, Aug 24, 2022 at 8:18 PM Jakub Kicinski wrote: > > We had historically not checked that genlmsghdr.reserved > is 0 on input which prevents us from using those precious > bytes in the future. > > One use case would be to extend the cmd field, which is > currently just 8 bits wide and 256 is not a lot of commands > for some core families. > > To make sure that new families do the right thing by default > put the onus of opting out of validation on existing families. > > Signed-off-by: Jakub Kicinski > --- > CC: j...@resnulli.us > CC: johan...@sipsolutions.net > CC: linux-bl...@vger.kernel.org > CC: osmocom-net-g...@lists.osmocom.org > CC: linux-w...@vger.kernel.org > CC: wireguard@lists.zx2c4.com > CC: linux-wirel...@vger.kernel.org > CC: linux-s...@vger.kernel.org > CC: target-de...@vger.kernel.org > CC: linux...@vger.kernel.org > CC: virtualizat...@lists.linux-foundation.org > CC: linux-c...@vger.kernel.org > CC: cluster-de...@redhat.com > CC: mp...@lists.linux.dev > CC: lvs-de...@vger.kernel.org > CC: netfilter-de...@vger.kernel.org > CC: linux-security-mod...@vger.kernel.org > CC: d...@openvswitch.org > CC: linux-s...@vger.kernel.org > CC: tipc-discuss...@lists.sourceforge.net > --- > drivers/block/nbd.c | 1 + > drivers/net/gtp.c| 1 + > drivers/net/ieee802154/mac802154_hwsim.c | 1 + > drivers/net/macsec.c | 1 + > drivers/net/team/team.c | 1 + > drivers/net/wireguard/netlink.c | 1 + > drivers/net/wireless/mac80211_hwsim.c| 1 + > drivers/target/target_core_user.c| 1 + > drivers/thermal/thermal_netlink.c| 1 + > drivers/vdpa/vdpa.c | 1 + > fs/cifs/netlink.c| 1 + > fs/dlm/netlink.c | 1 + > fs/ksmbd/transport_ipc.c | 1 + > include/linux/genl_magic_func.h | 1 + > include/net/genetlink.h | 3 +++ > kernel/taskstats.c | 1 + > net/batman-adv/netlink.c | 1 + > net/core/devlink.c | 1 + > net/core/drop_monitor.c | 1 + > net/ethtool/netlink.c| 1 + > net/hsr/hsr_netlink.c| 1 + > net/ieee802154/netlink.c | 1 + > net/ieee802154/nl802154.c| 1 + > net/ipv4/fou.c | 1 + > net/ipv4/tcp_metrics.c | 1 + > net/ipv6/ila/ila_main.c | 1 + > net/ipv6/ioam6.c | 1 + > net/ipv6/seg6.c | 1 + > net/l2tp/l2tp_netlink.c | 1 + > net/mptcp/pm_netlink.c | 1 + > net/ncsi/ncsi-netlink.c | 1 + > net/netfilter/ipvs/ip_vs_ctl.c | 1 + > net/netlabel/netlabel_calipso.c | 1 + > net/netlabel/netlabel_cipso_v4.c | 1 + > net/netlabel/netlabel_mgmt.c | 1 + > net/netlabel/netlabel_unlabeled.c| 1 + > net/netlink/genetlink.c | 4 > net/nfc/netlink.c| 1 + > net/openvswitch/conntrack.c | 1 + > net/openvswitch/datapath.c | 3 +++ > net/openvswitch/meter.c | 1 + > net/psample/psample.c| 1 + > net/smc/smc_netlink.c| 3 ++- > net/smc/smc_pnet.c | 3 ++- > net/tipc/netlink.c | 1 + > net/tipc/netlink_compat.c| 1 + > net/wireless/nl80211.c | 1 + > 47 files changed, 56 insertions(+), 2 deletions(-) Acked-by: Paul Moore (NetLabel) -- paul-moore.com
Re: [PATCH net-next] genetlink: start to validate reserved header bytes
Hello: This patch was applied to netdev/net-next.git (master) by David S. Miller : On Wed, 24 Aug 2022 17:18:30 -0700 you wrote: > We had historically not checked that genlmsghdr.reserved > is 0 on input which prevents us from using those precious > bytes in the future. > > One use case would be to extend the cmd field, which is > currently just 8 bits wide and 256 is not a lot of commands > for some core families. > > [...] Here is the summary with links: - [net-next] genetlink: start to validate reserved header bytes https://git.kernel.org/netdev/net-next/c/9c5d03d36251 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
[PATCH net-next] genetlink: start to validate reserved header bytes
We had historically not checked that genlmsghdr.reserved
is 0 on input which prevents us from using those precious
bytes in the future.
One use case would be to extend the cmd field, which is
currently just 8 bits wide and 256 is not a lot of commands
for some core families.
To make sure that new families do the right thing by default
put the onus of opting out of validation on existing families.
Signed-off-by: Jakub Kicinski
---
CC: j...@resnulli.us
CC: johan...@sipsolutions.net
CC: linux-bl...@vger.kernel.org
CC: osmocom-net-g...@lists.osmocom.org
CC: linux-w...@vger.kernel.org
CC: wireguard@lists.zx2c4.com
CC: linux-wirel...@vger.kernel.org
CC: linux-s...@vger.kernel.org
CC: target-de...@vger.kernel.org
CC: linux...@vger.kernel.org
CC: virtualizat...@lists.linux-foundation.org
CC: linux-c...@vger.kernel.org
CC: cluster-de...@redhat.com
CC: mp...@lists.linux.dev
CC: lvs-de...@vger.kernel.org
CC: netfilter-de...@vger.kernel.org
CC: linux-security-mod...@vger.kernel.org
CC: d...@openvswitch.org
CC: linux-s...@vger.kernel.org
CC: tipc-discuss...@lists.sourceforge.net
---
drivers/block/nbd.c | 1 +
drivers/net/gtp.c| 1 +
drivers/net/ieee802154/mac802154_hwsim.c | 1 +
drivers/net/macsec.c | 1 +
drivers/net/team/team.c | 1 +
drivers/net/wireguard/netlink.c | 1 +
drivers/net/wireless/mac80211_hwsim.c| 1 +
drivers/target/target_core_user.c| 1 +
drivers/thermal/thermal_netlink.c| 1 +
drivers/vdpa/vdpa.c | 1 +
fs/cifs/netlink.c| 1 +
fs/dlm/netlink.c | 1 +
fs/ksmbd/transport_ipc.c | 1 +
include/linux/genl_magic_func.h | 1 +
include/net/genetlink.h | 3 +++
kernel/taskstats.c | 1 +
net/batman-adv/netlink.c | 1 +
net/core/devlink.c | 1 +
net/core/drop_monitor.c | 1 +
net/ethtool/netlink.c| 1 +
net/hsr/hsr_netlink.c| 1 +
net/ieee802154/netlink.c | 1 +
net/ieee802154/nl802154.c| 1 +
net/ipv4/fou.c | 1 +
net/ipv4/tcp_metrics.c | 1 +
net/ipv6/ila/ila_main.c | 1 +
net/ipv6/ioam6.c | 1 +
net/ipv6/seg6.c | 1 +
net/l2tp/l2tp_netlink.c | 1 +
net/mptcp/pm_netlink.c | 1 +
net/ncsi/ncsi-netlink.c | 1 +
net/netfilter/ipvs/ip_vs_ctl.c | 1 +
net/netlabel/netlabel_calipso.c | 1 +
net/netlabel/netlabel_cipso_v4.c | 1 +
net/netlabel/netlabel_mgmt.c | 1 +
net/netlabel/netlabel_unlabeled.c| 1 +
net/netlink/genetlink.c | 4
net/nfc/netlink.c| 1 +
net/openvswitch/conntrack.c | 1 +
net/openvswitch/datapath.c | 3 +++
net/openvswitch/meter.c | 1 +
net/psample/psample.c| 1 +
net/smc/smc_netlink.c| 3 ++-
net/smc/smc_pnet.c | 3 ++-
net/tipc/netlink.c | 1 +
net/tipc/netlink_compat.c| 1 +
net/wireless/nl80211.c | 1 +
47 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/drivers/block/nbd.c b/drivers/block/nbd.c
index 2a709daefbc4..6cec9ce23fd3 100644
--- a/drivers/block/nbd.c
+++ b/drivers/block/nbd.c
@@ -2322,6 +2322,7 @@ static struct genl_family nbd_genl_family __ro_after_init
= {
.module = THIS_MODULE,
.small_ops = nbd_connect_genl_ops,
.n_small_ops= ARRAY_SIZE(nbd_connect_genl_ops),
+ .resv_start_op = NBD_CMD_STATUS + 1,
.maxattr= NBD_ATTR_MAX,
.policy = nbd_attr_policy,
.mcgrps = nbd_mcast_grps,
diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
index a208e2b1a9af..15c7dc82107f 100644
--- a/drivers/net/gtp.c
+++ b/drivers/net/gtp.c
@@ -1859,6 +1859,7 @@ static struct genl_family gtp_genl_family __ro_after_init
= {
.module = THIS_MODULE,
.small_ops = gtp_genl_ops,
.n_small_ops= ARRAY_SIZE(gtp_genl_ops),
+ .resv_start_op = GTP_CMD_ECHOREQ + 1,
.mcgrps = gtp_genl_mcgrps,
.n_mcgrps = ARRAY_SIZE(gtp_genl_mcgrps),
};
diff --git a/drivers/net/ieee802154/mac802154_hwsim.c
b/drivers/net/ieee802154/mac802154_hwsim.c
index 38c217bd7c82..2f0544dd7c2a 100644
--- a/drivers/net/ieee802154/mac802154_hwsim.c
+++ b/drivers/net/ieee802154/mac802154_hwsim.c
@@ -630,6 +630,7 @@ static struct genl_family hwsim_genl_family __ro_after_init
= {
.module = THIS_MODULE,
.small_ops = hwsim_nl_ops,
.n_small_ops = ARRAY_SIZE(hwsim_nl_ops),
+ .resv_start_op = MAC802154_HWSIM_CMD_NEW_EDGE + 1,
.mcgrps = hwsim_mcgrps,
.n_mcgrps
