| Message ID | 1615953763-23824-1-git-send-email-wenxu@ucloud.cn (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | afa536d8405a9ca36e45ba035554afbb8da27b82 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] net/sched: cls_flower: fix only mask bit check in the validate_ct_state | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | fail | 1 blamed authors not CCed: marcelo.leitner@gmail.com; 3 maintainers not CCed: xiyou.wangcong@gmail.com jiri@resnulli.us marcelo.leitner@gmail.com |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 88 this patch: 88 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 8 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 88 this patch: 88 |
| netdev/header_inline | success | Link |
On Wed, Mar 17, 2021 at 12:02:43PM +0800, wenxu@ucloud.cn wrote: > From: wenxu <wenxu@ucloud.cn> > > The ct_state validate should not only check the mask bit and also > check mask_bit & key_bit.. > For the +new+est case example, The 'new' and 'est' bits should be > set in both state_mask and state flags. Or the -new-est case also > will be reject by kernel. > When Openvswitch with two flows > ct_state=+trk+new,action=commit,forward > ct_state=+trk+est,action=forward > > A packet go through the kernel and the contrack state is invalid, > The ct_state will be +trk-inv. Upcall to the ovs-vswitchd, the > finally dp action will be drop with -new-est+trk. > > Fixes: 1bcc51ac0731 ("net/sched: cls_flower: Reject invalid ct_state flags rules") > Fixes: 3aed8b63336c ("net/sched: cls_flower: validate ct_state for invalid and reply flags") > Signed-off-by: wenxu <wenxu@ucloud.cn> > --- > net/sched/cls_flower.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c > index d097b5c..c69a4ba 100644 > --- a/net/sched/cls_flower.c > +++ b/net/sched/cls_flower.c > @@ -1451,7 +1451,7 @@ static int fl_set_key_ct(struct nlattr **tb, > &mask->ct_state, TCA_FLOWER_KEY_CT_STATE_MASK, > sizeof(key->ct_state)); > > - err = fl_validate_ct_state(mask->ct_state, > + err = fl_validate_ct_state(key->ct_state & mask->ct_state, Or that, yes. The thing I was wondering on this is if it would be a problem to have something like key = trk,inv mask = trk,new,est,inv because in essence, this is +trk+inv-new-est, and it's worrying about bits that shouldn't be considered if +inv is there. I don't see a reason for it to be that restrictive, though, and it will work as expected. Reviewed-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> > tb[TCA_FLOWER_KEY_CT_STATE_MASK], > extack); > if (err) > -- > 1.8.3.1 >
Hello: This patch was applied to netdev/net.git (refs/heads/master): On Wed, 17 Mar 2021 12:02:43 +0800 you wrote: > From: wenxu <wenxu@ucloud.cn> > > The ct_state validate should not only check the mask bit and also > check mask_bit & key_bit.. > For the +new+est case example, The 'new' and 'est' bits should be > set in both state_mask and state flags. Or the -new-est case also > will be reject by kernel. > When Openvswitch with two flows > ct_state=+trk+new,action=commit,forward > ct_state=+trk+est,action=forward > > [...] Here is the summary with links: - [net,v2] net/sched: cls_flower: fix only mask bit check in the validate_ct_state https://git.kernel.org/netdev/net/c/afa536d8405a You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c index d097b5c..c69a4ba 100644 --- a/net/sched/cls_flower.c +++ b/net/sched/cls_flower.c @@ -1451,7 +1451,7 @@ static int fl_set_key_ct(struct nlattr **tb, &mask->ct_state, TCA_FLOWER_KEY_CT_STATE_MASK, sizeof(key->ct_state)); - err = fl_validate_ct_state(mask->ct_state, + err = fl_validate_ct_state(key->ct_state & mask->ct_state, tb[TCA_FLOWER_KEY_CT_STATE_MASK], extack); if (err)