On Thu, Sep 06, 2018 at 06:34:00AM -0400, Jamal Hadi Salim wrote: > On 2018-09-06 6:28 a.m., Jamal Hadi Salim wrote: > > On 2018-09-05 3:04 p.m., Al Viro wrote: > > > From: Al Viro <v...@zeniv.linux.org.uk> > > > > > > ... and disallow deleting or linking to such > > > > > > Signed-off-by: Al Viro <v...@zeniv.linux.org.uk> > > > > Same comment as other one in regards to subject > > > > Since the flag space is coming from htnode which is > > exposed via uapi it makes sense to keep this one here > > because it is for private use; but a comment in > > include/uapi/linux/pkt_cls.h that this flag or > > maybe a set of bits is reserved for internal use. > > Otherwise: > > > > Acked-by: Jamal Hadi Salim <j...@mojatatu.com> > > > Sorry, additional comment: > It makes sense to reject user space attempt to > set TCA_CLS_FLAGS_U32_ROOT
Point, and that one is IMO enough to give up on using ->flags for that. How about simply diff --git a/net/sched/cls_u32.c b/net/sched/cls_u32.c index 3f985f29ef30..d14048e38b5c 100644 --- a/net/sched/cls_u32.c +++ b/net/sched/cls_u32.c @@ -84,6 +84,7 @@ struct tc_u_hnode { int refcnt; unsigned int divisor; struct idr handle_idr; + bool is_root; struct rcu_head rcu; u32 flags; /* The 'ht' field MUST be the last field in structure to allow for @@ -377,6 +378,7 @@ static int u32_init(struct tcf_proto *tp) root_ht->refcnt++; root_ht->handle = tp_c ? gen_new_htid(tp_c, root_ht) : 0x80000000; root_ht->prio = tp->prio; + root_ht->is_root = true; idr_init(&root_ht->handle_idr); if (tp_c == NULL) { @@ -693,7 +695,7 @@ static int u32_delete(struct tcf_proto *tp, void *arg, bool *last, goto out; } - if (root_ht == ht) { + if (ht->is_root) { NL_SET_ERR_MSG_MOD(extack, "Not allowed to delete root node"); return -EINVAL; } @@ -795,6 +797,10 @@ static int u32_set_parms(struct net *net, struct tcf_proto *tp, NL_SET_ERR_MSG_MOD(extack, "Link hash table not found"); return -EINVAL; } + if (ht_down->is_root) { + NL_SET_ERR_MSG_MOD(extack, "Not linking to root node"); + return -EINVAL; + } ht_down->refcnt++; }