diff mbox series

[net-next,v2,2/4] net/sched: act_pedit: check static offsets a priori

Message ID 20230314202448.603841-3-pctammela@mojatatu.com (mailing list archive)
State Superseded
Delegated to: Netdev Maintainers
Headers show
Series net/sched: act_pedit: minor improvements | expand

Checks

Context Check Description
netdev/series_format success Posting correctly formatted
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 18 this patch: 18
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 18 this patch: 18
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 18 this patch: 18
netdev/checkpatch warning WARNING: line length of 96 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Pedro Tammela March 14, 2023, 8:24 p.m. UTC 
Static key offsets should always be on 32 bit boundaries. Validate them on
create/update time for static offsets and move the datapath validation
for runtime offsets only.

iproute2 already errors out if a given offset and data size cannot be packed
to a 32 bit boundary. This change will make sure users which create/update pedit
instances directly via netlink also error out, instead of finding out
when packets are traversing.

Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>
---
 net/sched/act_pedit.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

Comments

Simon Horman March 15, 2023, 3:52 p.m. UTC | #1 
On Tue, Mar 14, 2023 at 05:24:46PM -0300, Pedro Tammela wrote:
> Static key offsets should always be on 32 bit boundaries. Validate them on
> create/update time for static offsets and move the datapath validation
> for runtime offsets only.
> 
> iproute2 already errors out if a given offset and data size cannot be packed
> to a 32 bit boundary. This change will make sure users which create/update pedit
> instances directly via netlink also error out, instead of finding out
> when packets are traversing.
> 
> Reviewed-by: Jamal Hadi Salim <jhs@mojatatu.com>
> Signed-off-by: Pedro Tammela <pctammela@mojatatu.com>

Reviewed-by: Simon Horman <simon.horman@corigine.com>
diff mbox series

Patch

diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c
index be9e7e565551..e6725e329ec1 100644
--- a/net/sched/act_pedit.c
+++ b/net/sched/act_pedit.c
@@ -249,6 +249,12 @@  static int tcf_pedit_init(struct net *net, struct nlattr *nla,
 	for (i = 0; i < nparms->tcfp_nkeys; ++i) {
 		u32 cur = nparms->tcfp_keys[i].off;
 
+		if (cur % 4) {
+			NL_SET_ERR_MSG_MOD(extack, "Pedit offsets must be on 32bit boundaries");
+			ret = -EINVAL;
+			goto put_chain;
+		}
+
 		/* sanitize the shift value for any later use */
 		nparms->tcfp_keys[i].shift = min_t(size_t,
 						   BITS_PER_TYPE(int) - 1,
@@ -407,12 +413,12 @@  TC_INDIRECT_SCOPE int tcf_pedit_act(struct sk_buff *skb,
 					       sizeof(_d), &_d);
 			if (!d)
 				goto bad;
-			offset += (*d & tkey->offmask) >> tkey->shift;
-		}
 
-		if (offset % 4) {
-			pr_info("tc action pedit offset must be on 32 bit boundaries\n");
-			goto bad;
+			offset += (*d & tkey->offmask) >> tkey->shift;
+			if (offset % 4) {
+				pr_info("tc action pedit offset must be on 32 bit boundaries\n");
+				goto bad;
+			}
 		}
 
 		if (!offset_valid(skb, hoffset + offset)) {