On Mon, Apr 08, 2024 at 02:55:21PM +0200, Greg Kroah-Hartman wrote:
> 6.8-stable review patch. If anyone has any objections, please let me know.
>
> ------------------
>
> From: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
>
> [ Upstream commit 1e1fb6f00f52812277963365d9bd835b9b0ea4e0 ]
>
> netdev basechain updates are stored in the transaction object hook list.
> When setting on the table dormant flag, it iterates over the existing
> hooks in the basechain. Thus, skipping the hooks that are being
> added/deleted in this transaction, which leaves hook registration in
> inconsistent state.
>
> Reject table flag updates in combination with netdev basechain updates
> in the same batch:
>
> - Update table flags and add/delete basechain: Check from basechain update
> path if there are pending flag updates for this table.
> - add/delete basechain and update table flags: Iterate over the transaction
> list to search for basechain updates from the table update path.
>
> In both cases, the batch is rejected. Based on suggestion from Florian Westphal.
>
> Fixes: b9703ed44ffb ("netfilter: nf_tables: support for adding new devices to an existing netdev chain")
> Fixes: 7d937b107108f ("netfilter: nf_tables: support for deleting devices in an existing netdev chain")
> Signed-off-by: Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx>
> Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx>
> ---
> net/netfilter/nf_tables_api.c | 31 ++++++++++++++++++++++++++++++-
> 1 file changed, 30 insertions(+), 1 deletion(-)
>
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 00288b31f734c..db233965631bb 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -1198,6 +1198,25 @@ static void nf_tables_table_disable(struct net *net, struct nft_table *table)
> #define __NFT_TABLE_F_UPDATE (__NFT_TABLE_F_WAS_DORMANT | \
> __NFT_TABLE_F_WAS_AWAKEN)
>
> +static bool nft_table_pending_update(const struct nft_ctx *ctx)
> +{
> + struct nftables_pernet *nft_net = nft_pernet(ctx->net);
> + struct nft_trans *trans;
> +
> + if (ctx->table->flags & __NFT_TABLE_F_UPDATE)
> + return true;
> +
> + list_for_each_entry(trans, &nft_net->commit_list, list) {
> + if ((trans->msg_type == NFT_MSG_NEWCHAIN ||
^.........................................^
Remove this, only update is narrowed down.
Thanks.
> + trans->msg_type == NFT_MSG_DELCHAIN) &&
> + trans->ctx.table == ctx->table &&
> + nft_trans_chain_update(trans))
> + return true;
> + }
> +
> + return false;
> +}
> +
> static int nf_tables_updtable(struct nft_ctx *ctx)
> {
> struct nft_trans *trans;
> @@ -1221,7 +1240,7 @@ static int nf_tables_updtable(struct nft_ctx *ctx)
> return -EOPNOTSUPP;
>
> /* No dormant off/on/off/on games in single transaction */
> - if (ctx->table->flags & __NFT_TABLE_F_UPDATE)
> + if (nft_table_pending_update(ctx))
> return -EINVAL;
>
> trans = nft_trans_alloc(ctx, NFT_MSG_NEWTABLE,
> @@ -2619,6 +2638,13 @@ static int nf_tables_updchain(struct nft_ctx *ctx, u8 genmask, u8 policy,
> }
> }
>
> + if (table->flags & __NFT_TABLE_F_UPDATE &&
> + !list_empty(&hook.list)) {
> + NL_SET_BAD_ATTR(extack, attr);
> + err = -EOPNOTSUPP;
> + goto err_hooks;
> + }
> +
> if (!(table->flags & NFT_TABLE_F_DORMANT) &&
> nft_is_base_chain(chain) &&
> !list_empty(&hook.list)) {
> @@ -2848,6 +2874,9 @@ static int nft_delchain_hook(struct nft_ctx *ctx,
> struct nft_trans *trans;
> int err;
>
> + if (ctx->table->flags & __NFT_TABLE_F_UPDATE)
> + return -EOPNOTSUPP;
> +
> err = nft_chain_parse_hook(ctx->net, basechain, nla, &chain_hook,
> ctx->family, chain->flags, extack);
> if (err < 0)
> --
> 2.43.0
>
>
>
