From nobody Sat Apr 20 12:59:37 2024 Delivered-To: wpasupplicant.patchew@gmail.com Received: by 2002:a05:6a06:869:b0:4b8:7781:bd2f with SMTP id d41csp1678360pis; Fri, 6 May 2022 02:21:31 -0700 (PDT) X-Google-Smtp-Source: ABdhPJxxO+YMRgoiKZP9v49UXgBD76pddiXWZ2t/3xoPv0UPb5bskLHUVF8toF1Z9BKmGehXw0NA X-Received: by 2002:a17:90b:1d03:b0:1dc:db97:942c with SMTP id on3-20020a17090b1d0300b001dcdb97942cmr2178509pjb.238.1651828890949; Fri, 06 May 2022 02:21:30 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1651828890; cv=none; d=google.com; s=arc-20160816; b=WEZ/+Or+riYoA0C71BqRxWoi1A5VcMl72lKulke1mScuoqeMNQuMcUyU8FP6PJibXT yqjr+IvzkW7mjTwB82q69+5xj788r7jmABbHNPPcZg4G/2aK0Nw82UIoSbMygdrymCgo crWW0MwVH5mwIVtdlyLLhbwrzInycyU28aIKqpMT9rkqMaF9hxqti0xOjMn6/lOY6U4A 3214Vd+mS4BcxgBWbNy8OEsiHW49dt5GWYecMTka7+qGdmOcF5zebjqGxb7bTF9g7L4G K+smav50m/Jbqfmokqho2qFU1qaFcxEzH3j1AiW7PuCsZ4C96uWGybQcwzgl+26fKvJU hvPA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=content-transfer-encoding:mime-version:list-unsubscribe :list-subscribe:list-id:precedence:message-id:date:subject:to:from :dkim-signature; bh=WWJwwH7Z7c6I4hQNbK34IQbG6et0V0n7VjO7O5IA+kg=; b=Siv+3IbRI0mJv1nseLLDLNqZUcJV8nbpVo+I7nHqSQ5ld77bpBOklFY+ZjBwyotpaA feKgIzNBHhcZPVz0UOqlvPf6MizDF0kkUrlKIFyNCUb4fJXLHlQybPqsq81mMe2XaBof /cm1lxj5r8n333sEjhyzYrmzxL0Btbv7CBIX9jfiGSIsgJAP0FTC98XGwtXFy6yS7TqR fIZ78WrVk71A5utO/g9Oa+Cftr7nL4QcYEOQZrQGAuSQxhsf2IcngzmNIm6pjAZ1iZ5x saK9vCoSuZxCniOOZNrziER9V3YvXfRsPbXtS2RIlwf6bOLB8M1+nEDU0p6Elz0iZkhB +ZOg== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b="CwxWWlj/"; spf=pass (google.com: domain of mptcp+bounces-5152-wpasupplicant.patchew=gmail.com@lists.linux.dev designates 139.178.88.99 as permitted sender) smtp.mailfrom="mptcp+bounces-5152-wpasupplicant.patchew=gmail.com@lists.linux.dev"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Return-Path: Received: from sv.mirrors.kernel.org (sv.mirrors.kernel.org. [139.178.88.99]) by mx.google.com with ESMTPS id d18-20020a056a0024d200b004fa843103dbsi4727227pfv.193.2022.05.06.02.21.30 for (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Fri, 06 May 2022 02:21:30 -0700 (PDT) Received-SPF: pass (google.com: domain of mptcp+bounces-5152-wpasupplicant.patchew=gmail.com@lists.linux.dev designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99; Authentication-Results: mx.google.com; dkim=pass header.i=@redhat.com header.s=mimecast20190719 header.b="CwxWWlj/"; spf=pass (google.com: domain of mptcp+bounces-5152-wpasupplicant.patchew=gmail.com@lists.linux.dev designates 139.178.88.99 as permitted sender) smtp.mailfrom="mptcp+bounces-5152-wpasupplicant.patchew=gmail.com@lists.linux.dev"; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=redhat.com Received: from smtp.subspace.kernel.org (wormhole.subspace.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by sv.mirrors.kernel.org (Postfix) with ESMTPS id 6BF2F2808FB for ; Fri, 6 May 2022 09:21:30 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 2232A7F8; Fri, 6 May 2022 09:21:29 +0000 (UTC) X-Original-To: mptcp@lists.linux.dev Received: from us-smtp-delivery-74.mimecast.com (us-smtp-delivery-74.mimecast.com [170.10.129.74]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 99CD97F4 for ; Fri, 6 May 2022 09:21:27 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1651828886; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=WWJwwH7Z7c6I4hQNbK34IQbG6et0V0n7VjO7O5IA+kg=; b=CwxWWlj/xIsq2XKa2wuzNPCXPS7i6CJNrsnLEJOXAHcy8QkcN0MqukEd7RYT5ADNmVTQir 0dmwLI41+4VcbXPY+rrM9a+bEVueGVEFUTHIZKwTXsKSOyfkGGBDiQFpbXKRJrX1ueMwWF tc0DY2mJGbbBhLhmLonLj5+bZM0bCRI= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-609-WlylO9f0Nem_49ejmk0yNA-1; Fri, 06 May 2022 05:21:25 -0400 X-MC-Unique: WlylO9f0Nem_49ejmk0yNA-1 Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id E424338C5C46 for ; Fri, 6 May 2022 09:21:24 +0000 (UTC) Received: from gerbillo.redhat.com (unknown [10.39.195.157]) by smtp.corp.redhat.com (Postfix) with ESMTP id 72292428F11 for ; Fri, 6 May 2022 09:21:24 +0000 (UTC) From: Paolo Abeni To: mptcp@lists.linux.dev Subject: [PATCH net v3] net/sched: act_pedit: really ensure the skb is writable Date: Fri, 6 May 2022 11:21:17 +0200 Message-Id: <7ec2ff1bd88c6d87e452afa58f1f6b6c756a2fb0.1651828857.git.pabeni@redhat.com> Precedence: bulk X-Mailing-List: mptcp@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.9 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=pabeni@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8"; x-default="true" Currently pedit tries to ensure that the accessed skb offset is writeble via skb_unclone(). The action potentially allows touching any skb bytes, so it may end-up modifying shared data. The above causes some sporadic MPTCP self-test failures. Address the issue keeping track of a rough over-estimate highest skb offset accessed by the action and ensure such offset is really writable. Note that this may cause performance regressions in some scenario, but hopefully pedit is not critical path. v1 -> v2: - cleanup hint update (Jakub) - avoid raices while accessing the hint (Jakub) - re-organize the comments for clarity Fixes: db2c24175d14 ("act_pedit: access skb->data safely") Acked-by: Mat Martineau Tested-by: Geliang Tang Signed-off-by: Paolo Abeni --- v2 -> v3: - release lock on error (Mat) note: part of the changelog is here, because -net will see only v2 (hopefully) --- include/net/tc_act/tc_pedit.h | 1 + net/sched/act_pedit.c | 26 ++++++++++++++++++++++---- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/include/net/tc_act/tc_pedit.h b/include/net/tc_act/tc_pedit.h index 748cf87a4d7e..3e02709a1df6 100644 --- a/include/net/tc_act/tc_pedit.h +++ b/include/net/tc_act/tc_pedit.h @@ -14,6 +14,7 @@ struct tcf_pedit { struct tc_action common; unsigned char tcfp_nkeys; unsigned char tcfp_flags; + u32 tcfp_off_max_hint; struct tc_pedit_key *tcfp_keys; struct tcf_pedit_key_ex *tcfp_keys_ex; }; diff --git a/net/sched/act_pedit.c b/net/sched/act_pedit.c index e01ef7f109f4..d1221daa0952 100644 --- a/net/sched/act_pedit.c +++ b/net/sched/act_pedit.c @@ -149,7 +149,7 @@ static int tcf_pedit_init(struct net *net, struct nlatt= r *nla, struct nlattr *pattr; struct tcf_pedit *p; int ret =3D 0, err; - int ksize; + int i, ksize; u32 index; =20 if (!nla) { @@ -228,6 +228,18 @@ static int tcf_pedit_init(struct net *net, struct nlat= tr *nla, p->tcfp_nkeys =3D parm->nkeys; } memcpy(p->tcfp_keys, parm->keys, ksize); + p->tcfp_off_max_hint =3D 0; + for (i =3D 0; i < p->tcfp_nkeys; ++i) { + u32 cur =3D p->tcfp_keys[i].off; + + /* The AT option can read a single byte, we can bound the actual + * value with uchar max. + */ + cur +=3D (0xff & p->tcfp_keys[i].offmask) >> p->tcfp_keys[i].shift; + + /* Each key touches 4 bytes starting from the computed offset */ + p->tcfp_off_max_hint =3D max(p->tcfp_off_max_hint, cur + 4); + } =20 p->tcfp_flags =3D parm->flags; goto_ch =3D tcf_action_set_ctrlact(*a, parm->action, goto_ch); @@ -308,13 +320,18 @@ static int tcf_pedit_act(struct sk_buff *skb, const s= truct tc_action *a, struct tcf_result *res) { struct tcf_pedit *p =3D to_pedit(a); + u32 max_offset; int i; =20 - if (skb_unclone(skb, GFP_ATOMIC)) - return p->tcf_action; - spin_lock(&p->tcf_lock); =20 + max_offset =3D (skb_transport_header_was_set(skb) ? + skb_transport_offset(skb) : + skb_network_offset(skb)) + + p->tcfp_off_max_hint; + if (skb_ensure_writable(skb, min(skb->len, max_offset))) + goto unlock; + tcf_lastuse_update(&p->tcf_tm); =20 if (p->tcfp_nkeys > 0) { @@ -403,6 +420,7 @@ static int tcf_pedit_act(struct sk_buff *skb, const str= uct tc_action *a, p->tcf_qstats.overlimits++; done: bstats_update(&p->tcf_bstats, skb); +unlock: spin_unlock(&p->tcf_lock); return p->tcf_action; } --=20 2.35.1