Message ID | 20240220-raw-setsockopt-v1-1-7d34cb1377fc@pengutronix.de |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel+bounces-72520-ouuuleilei=gmail.com@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:693c:2685:b0:108:e6aa:91d0 with SMTP id mn5csp258063dyc; Tue, 20 Feb 2024 00:16:43 -0800 (PST) X-Forwarded-Encrypted: i=3; AJvYcCXF1CQlc5i0fHgKsoD3+ZxbwRl+QbR4YBDbfjGPwqPy5crZ+0grb/5NVzjdFmDLRBtdUSHxpD8dpyMz9PfxwBhByNiscA== X-Google-Smtp-Source: AGHT+IFZqNBjWKDGDBm/Du+9zyGU1NkFSO8sWg0No7jC/2Wia6SjS+jlJeLzbPOU4sRBnLBIS6GA X-Received: by 2002:a05:620a:22d1:b0:787:7ca8:2005 with SMTP id o17-20020a05620a22d100b007877ca82005mr303594qki.76.1708417003636; Tue, 20 Feb 2024 00:16:43 -0800 (PST) ARC-Seal: i=2; a=rsa-sha256; t=1708417003; cv=pass; d=google.com; s=arc-20160816; b=EKXfSSKjgGXlKw1x/99TW/mqwMbIlVrda4UorlmsGI9W2n2GRGbEqND+x/PRo9H6c2 fGkMQpHFXVZJJOmbpnS17qb1F7jdvuh5tWkZYJFb2hONAGVX5oPbO+lv3zx+gDk+nCjM M0598HO2P3G1Myd8183o096HettK4xbgjmcJWYZ4NinEQwUPLRxdfoxZoBAGw097H16W BrtdYts0yIJhRuIk47p0Uq8Gv0gAPZnqOAeN/k6wDZpY5dH+LL6BzbV04nvYs/d+PEnE UP0825sV5n0TpVwaNZud6gtnbG0RWh0BmQGIDJg/HL33PJLPuTC8N/Bn10kV4Tneu/c2 DiOQ== ARC-Message-Signature: i=2; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=cc:to:message-id:content-transfer-encoding:mime-version :list-unsubscribe:list-subscribe:list-id:precedence:subject:date :from; bh=fr/9GyRGkEj5yWJ7N73Np8PCHiMz3L3XpsIe3der4kE=; fh=AQjVssW+zhM/Be14OpzhaZMp6IxHHeWt09xZ24ery7U=; b=xhriusDqf3zSnFVNuJGDVPOlCDuo3JM8XmaGBlbhSUq5eXWml0Jv+7LmeEKbEDkI6j 8Ql9edgA6GMG9S3ltx91VczUeaeP7LO0YIX2efrh6KUk1bNdy+t41ybvf2EvtBF7WlTo sOIFNAzAiariBoILgMUfkzdjBj2X3qHhonIDTr6Q3UV7JKD306tqzjxYd3dvESNttB12 oYYZSlU+v4tcsHLTugohZriy0ls4wzu20ONpgQcwLD7WMbf+Pdu4xJAHu0cjepoWBfsq gSopbRrZmQC7uU5vpUbN8ADH2A+A2Miqf4jQ38H33YyYlfV14mjZCHeR0rA/6E0SypNg JSYA==; dara=google.com ARC-Authentication-Results: i=2; mx.google.com; arc=pass (i=1 spf=pass spfdomain=pengutronix.de); spf=pass (google.com: domain of linux-kernel+bounces-72520-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-72520-ouuuleilei=gmail.com@vger.kernel.org" Received: from ny.mirrors.kernel.org (ny.mirrors.kernel.org. [2604:1380:45d1:ec00::1]) by mx.google.com with ESMTPS id pj6-20020a05620a1d8600b00785ca459548si7823984qkn.113.2024.02.20.00.16.43 for <ouuuleilei@gmail.com> (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 20 Feb 2024 00:16:43 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel+bounces-72520-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1; Authentication-Results: mx.google.com; arc=pass (i=1 spf=pass spfdomain=pengutronix.de); spf=pass (google.com: domain of linux-kernel+bounces-72520-ouuuleilei=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) smtp.mailfrom="linux-kernel+bounces-72520-ouuuleilei=gmail.com@vger.kernel.org" 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 ny.mirrors.kernel.org (Postfix) with ESMTPS id 5A9361C22286 for <ouuuleilei@gmail.com>; Tue, 20 Feb 2024 08:16:43 +0000 (UTC) Received: from localhost.localdomain (localhost.localdomain [127.0.0.1]) by smtp.subspace.kernel.org (Postfix) with ESMTP id BAB015C8EC; Tue, 20 Feb 2024 08:16:29 +0000 (UTC) Received: from metis.whiteo.stw.pengutronix.de (metis.whiteo.stw.pengutronix.de [185.203.201.7]) (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 DB64A53819 for <linux-kernel@vger.kernel.org>; Tue, 20 Feb 2024 08:16:25 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=185.203.201.7 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708416988; cv=none; b=TwRqlc/xl1ymx8oAA9B8ZvRaykkvviBuZx69jwuAVtf4GRBSMLc7h+J6hk0TqRFO4D6poggLqqeUZuGXcyaLMaBUVMOudd6v3yF+7xHbhS3Ln7tFoY8zg0lxFUQTmbgrMfPfWv8LMD5GH7e4Ph+Z2hozhq64lJmJXhCwg7W/qBw= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1708416988; c=relaxed/simple; bh=g1EHi/WqOcNNnaGmzLYGdEE0tjJO8ATGZtEws8+sO2M=; h=From:Date:Subject:MIME-Version:Content-Type:Message-Id:To:Cc; b=D8LTtE+GIAmJ0jHFZ15VeQfbSs/4mo8fWFm/Snw9OPw/U2CT1hdsFWrBmusDrAWiBGB5E4Ss1kDyyXuMEI/xkDRmxBSb4eMxgPpRqOrx5ZDEXPCunNz08byB+bHQyvTfKSFWhljAqDK7rQPpkYL6wc2YD1/ZM821GbJsJu0FQj0= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de; spf=pass smtp.mailfrom=pengutronix.de; arc=none smtp.client-ip=185.203.201.7 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=pengutronix.de Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=pengutronix.de Received: from drehscheibe.grey.stw.pengutronix.de ([2a0a:edc0:0:c01:1d::a2]) by metis.whiteo.stw.pengutronix.de with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.92) (envelope-from <mkl@pengutronix.de>) id 1rcLIe-0003zU-Ax for linux-kernel@vger.kernel.org; Tue, 20 Feb 2024 09:16:24 +0100 Received: from [2a0a:edc0:0:b01:1d::7b] (helo=bjornoya.blackshift.org) by drehscheibe.grey.stw.pengutronix.de with esmtps (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from <mkl@pengutronix.de>) id 1rcLId-001o51-UO for linux-kernel@vger.kernel.org; Tue, 20 Feb 2024 09:16:23 +0100 Received: from dspam.blackshift.org (localhost [127.0.0.1]) by bjornoya.blackshift.org (Postfix) with SMTP id 9CD38292E29 for <linux-kernel@vger.kernel.org>; Tue, 20 Feb 2024 08:16:23 +0000 (UTC) Received: from hardanger.blackshift.org (unknown [172.20.34.65]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by bjornoya.blackshift.org (Postfix) with ESMTPS id ACA61292E21; Tue, 20 Feb 2024 08:16:21 +0000 (UTC) Received: from [172.20.34.65] (localhost [::1]) by hardanger.blackshift.org (OpenSMTPD) with ESMTP id aa76ddf2; Tue, 20 Feb 2024 08:16:21 +0000 (UTC) From: Marc Kleine-Budde <mkl@pengutronix.de> Date: Tue, 20 Feb 2024 09:16:16 +0100 Subject: [PATCH] can: raw: raw_getsockopt(): reduce scope of err Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: <linux-kernel.vger.kernel.org> List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org> List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Message-Id: <20240220-raw-setsockopt-v1-1-7d34cb1377fc@pengutronix.de> X-B4-Tracking: v=1; b=H4sIAM9f1GUC/x2MQQqAIBAAvxJ7TrAtCvpKdDDbaglUXKkg/HvS3 OYw84JQZBIYqxciXSzsXZGmrsAexu2keC0OqLHTiFpFcyuhJN6ePiS19dQMrV2XApQoRNr4+Yf TnPMHDt2Qr2AAAAA= To: Oliver Hartkopp <socketcan@hartkopp.net>, "David S. Miller" <davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com> Cc: linux-can@vger.kernel.org, netdev@vger.kernel.org, linux-kernel@vger.kernel.org, Marc Kleine-Budde <mkl@pengutronix.de> X-Mailer: b4 0.14-dev X-Developer-Signature: v=1; a=openpgp-sha256; l=2005; i=mkl@pengutronix.de; h=from:subject:message-id; bh=g1EHi/WqOcNNnaGmzLYGdEE0tjJO8ATGZtEws8+sO2M=; b=owEBbQGS/pANAwAKASg4oj56LbxvAcsmYgBl1F/Se2ksHfZe5EBj6QKPxoRNOdr4xE3O2uvaf MoZfkIDpPiJATMEAAEKAB0WIQRQQLqG4LYE3Sm8Pl8oOKI+ei28bwUCZdRf0gAKCRAoOKI+ei28 bw12B/9VO1K8vR6SVTcAv/NKuk7fv9U6p4KoxoZRXV9l6BbMgaALSKomtAGh+77Opq5qFvmnF+t IMe0UCYYuiX52ijD/EsPtSOC6Z0x4tkezqYRStZBxl58888eFAC07sXpIEnkL8c81HisIAQZyo6 L+Aqxt/cSZxOxJzZYWYJRjqZXpVGMnZQ8ykZw56VZteXejA3VmeBOkvakuGhC+wYe39ChdsxTuJ pgtZqPvvrfbym6f0QmZpfedFW6ulKffRlTUKulqT8079Sg1Nf6PkhZJH9QCnqzF1TyfJivJ9l2q WUiORXtLx0BJlD6t03nYNZug4ieMRDXFby+BORCSs3ylo7jm X-Developer-Key: i=mkl@pengutronix.de; a=openpgp; fpr=C1400BA0B3989E6FBC7D5B5C2B5EE211C58AEA54 X-SA-Exim-Connect-IP: 2a0a:edc0:0:c01:1d::a2 X-SA-Exim-Mail-From: mkl@pengutronix.de X-SA-Exim-Scanned: No (on metis.whiteo.stw.pengutronix.de); SAEximRunCond expanded to false X-PTX-Original-Recipient: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1791405067902158634 X-GMAIL-MSGID: 1791405067902158634 |
Series |
can: raw: raw_getsockopt(): reduce scope of err
|
|
Commit Message
Marc Kleine-Budde
Feb. 20, 2024, 8:16 a.m. UTC
Reduce the scope of the variable "err" to the individual cases. This
is to avoid the mistake of setting "err" in the mistaken belief that
it will be evaluated later.
Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de>
---
net/can/raw.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
---
base-commit: c8fba5d6df5e476aa791db4f1f014dad2bb5e904
change-id: 20240220-raw-setsockopt-f6e173cdbbbb
Best regards,
Comments
On Tue, Feb 20, 2024 at 9:16 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > Reduce the scope of the variable "err" to the individual cases. This > is to avoid the mistake of setting "err" in the mistaken belief that > it will be evaluated later. > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > net/can/raw.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/net/can/raw.c b/net/can/raw.c > index 897ffc17d850..2bb3eab98af0 100644 > --- a/net/can/raw.c > +++ b/net/can/raw.c > @@ -756,7 +756,6 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > struct raw_sock *ro = raw_sk(sk); > int len; > void *val; > - int err = 0; > > if (level != SOL_CAN_RAW) > return -EINVAL; > @@ -766,7 +765,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > return -EINVAL; > > switch (optname) { > - case CAN_RAW_FILTER: > + case CAN_RAW_FILTER: { > + int err = 0; > + > lock_sock(sk); > if (ro->count > 0) { > int fsize = ro->count * sizeof(struct can_filter); > @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > if (!err) > err = put_user(len, optlen); > return err; > - > + } > case CAN_RAW_ERR_FILTER: > if (len > sizeof(can_err_mask_t)) > len = sizeof(can_err_mask_t); > @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > val = &ro->xl_frames; > break; > > - case CAN_RAW_XL_VCID_OPTS: > + case CAN_RAW_XL_VCID_OPTS: { > + int err = 0; > + > /* user space buffer to small for VCID opts? */ > if (len < sizeof(ro->raw_vcid_opts)) { > /* return -ERANGE and needed space in optlen */ > @@ -839,6 +842,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > err = put_user(len, optlen); > return err; > > + } > case CAN_RAW_JOIN_FILTERS: > if (len > sizeof(int)) > len = sizeof(int); > > --- > base-commit: c8fba5d6df5e476aa791db4f1f014dad2bb5e904 > change-id: 20240220-raw-setsockopt-f6e173cdbbbb What is the target tree ? In net-next tree, syzbot complained about a bug added in commit c83c22ec1493c0b7cc77327bedbd387e295872b6 Author: Oliver Hartkopp <socketcan@hartkopp.net> Date: Mon Feb 12 22:35:50 2024 +0100 can: canxl: add virtual CAN network identifier support Patch to fix the issue has not been sent yet ? diff --git a/net/can/raw.c b/net/can/raw.c index cb8e6f788af84ac65830399baac6d1cf3d093e08..897ffc17d850670003e5cf3402477e8fc201f61e 100644 --- a/net/can/raw.c +++ b/net/can/raw.c @@ -835,7 +835,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, if (copy_to_user(optval, &ro->raw_vcid_opts, len)) err = -EFAULT; } - break; + if (!err) + err = put_user(len, optlen); + return err; case CAN_RAW_JOIN_FILTERS: if (len > sizeof(int))
On Tue. 20 Feb. 2024 at 17:16, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > Reduce the scope of the variable "err" to the individual cases. This > is to avoid the mistake of setting "err" in the mistaken belief that > it will be evaluated later. > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> One nitpick to remove a newline, but aside from that: Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> (as usual, I do not mind if the nitpick gets resolved while applying). > --- > net/can/raw.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/net/can/raw.c b/net/can/raw.c > index 897ffc17d850..2bb3eab98af0 100644 > --- a/net/can/raw.c > +++ b/net/can/raw.c > @@ -756,7 +756,6 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > struct raw_sock *ro = raw_sk(sk); > int len; > void *val; > - int err = 0; > > if (level != SOL_CAN_RAW) > return -EINVAL; > @@ -766,7 +765,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > return -EINVAL; > > switch (optname) { > - case CAN_RAW_FILTER: > + case CAN_RAW_FILTER: { Aesthetically speaking, I do not like turning the cases into compound statement, but this removes a pitfall, so let go for it! > + int err = 0; > + > lock_sock(sk); > if (ro->count > 0) { > int fsize = ro->count * sizeof(struct can_filter); > @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > if (!err) > err = put_user(len, optlen); > return err; > - > + } > case CAN_RAW_ERR_FILTER: > if (len > sizeof(can_err_mask_t)) > len = sizeof(can_err_mask_t); > @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > val = &ro->xl_frames; > break; > > - case CAN_RAW_XL_VCID_OPTS: > + case CAN_RAW_XL_VCID_OPTS: { > + int err = 0; > + > /* user space buffer to small for VCID opts? */ > if (len < sizeof(ro->raw_vcid_opts)) { > /* return -ERANGE and needed space in optlen */ > @@ -839,6 +842,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > err = put_user(len, optlen); > return err; > > + } Nitpick: to be aligned with the above, also remove the newline here (same as above): - + } > case CAN_RAW_JOIN_FILTERS: > if (len > sizeof(int)) > len = sizeof(int);
On 2024-02-20 09:25, Eric Dumazet wrote: > On Tue, Feb 20, 2024 at 9:16 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: >> >> Reduce the scope of the variable "err" to the individual cases. This >> is to avoid the mistake of setting "err" in the mistaken belief that >> it will be evaluated later. >> >> Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> >> --- >> net/can/raw.c | 12 ++++++++---- >> 1 file changed, 8 insertions(+), 4 deletions(-) >> >> diff --git a/net/can/raw.c b/net/can/raw.c >> index 897ffc17d850..2bb3eab98af0 100644 >> --- a/net/can/raw.c >> +++ b/net/can/raw.c >> @@ -756,7 +756,6 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, >> struct raw_sock *ro = raw_sk(sk); >> int len; >> void *val; >> - int err = 0; >> >> if (level != SOL_CAN_RAW) >> return -EINVAL; >> @@ -766,7 +765,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, >> return -EINVAL; >> >> switch (optname) { >> - case CAN_RAW_FILTER: >> + case CAN_RAW_FILTER: { >> + int err = 0; >> + >> lock_sock(sk); >> if (ro->count > 0) { >> int fsize = ro->count * sizeof(struct can_filter); >> @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, >> if (!err) >> err = put_user(len, optlen); >> return err; >> - >> + } >> case CAN_RAW_ERR_FILTER: >> if (len > sizeof(can_err_mask_t)) >> len = sizeof(can_err_mask_t); >> @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, >> val = &ro->xl_frames; >> break; >> >> - case CAN_RAW_XL_VCID_OPTS: >> + case CAN_RAW_XL_VCID_OPTS: { >> + int err = 0; >> + >> /* user space buffer to small for VCID opts? */ >> if (len < sizeof(ro->raw_vcid_opts)) { >> /* return -ERANGE and needed space in optlen */ >> @@ -839,6 +842,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, >> err = put_user(len, optlen); >> return err; >> >> + } >> case CAN_RAW_JOIN_FILTERS: >> if (len > sizeof(int)) >> len = sizeof(int); >> >> --- >> base-commit: c8fba5d6df5e476aa791db4f1f014dad2bb5e904 >> change-id: 20240220-raw-setsockopt-f6e173cdbbbb > > What is the target tree ? > > In net-next tree, syzbot complained about a bug added in > > commit c83c22ec1493c0b7cc77327bedbd387e295872b6 > Author: Oliver Hartkopp <socketcan@hartkopp.net> > Date: Mon Feb 12 22:35:50 2024 +0100 > > can: canxl: add virtual CAN network identifier support > > Patch to fix the issue has not been sent yet ? The patch with the fix has been sent yesterday and Marc applied it to his linux-can-next tree for a pull request to net-next. https://lore.kernel.org/linux-can/20240220-mobility-thigh-8ddfb02bfab9-mkl@pengutronix.de/T/#t Best regards, Oliver > > diff --git a/net/can/raw.c b/net/can/raw.c > index cb8e6f788af84ac65830399baac6d1cf3d093e08..897ffc17d850670003e5cf3402477e8fc201f61e > 100644 > --- a/net/can/raw.c > +++ b/net/can/raw.c > @@ -835,7 +835,9 @@ static int raw_getsockopt(struct socket *sock, int > level, int optname, > if (copy_to_user(optval, &ro->raw_vcid_opts, len)) > err = -EFAULT; > } > - break; > + if (!err) > + err = put_user(len, optlen); > + return err; > > case CAN_RAW_JOIN_FILTERS: > if (len > sizeof(int)) >
On 20.02.2024 09:25:58, Eric Dumazet wrote: > On Tue, Feb 20, 2024 at 9:16 AM Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > > > Reduce the scope of the variable "err" to the individual cases. This > > is to avoid the mistake of setting "err" in the mistaken belief that > > it will be evaluated later. > > > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > --- > > net/can/raw.c | 12 ++++++++---- > > 1 file changed, 8 insertions(+), 4 deletions(-) > > > > diff --git a/net/can/raw.c b/net/can/raw.c > > index 897ffc17d850..2bb3eab98af0 100644 > > --- a/net/can/raw.c > > +++ b/net/can/raw.c > > @@ -756,7 +756,6 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > > struct raw_sock *ro = raw_sk(sk); > > int len; > > void *val; > > - int err = 0; > > > > if (level != SOL_CAN_RAW) > > return -EINVAL; > > @@ -766,7 +765,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > > return -EINVAL; > > > > switch (optname) { > > - case CAN_RAW_FILTER: > > + case CAN_RAW_FILTER: { > > + int err = 0; > > + > > lock_sock(sk); > > if (ro->count > 0) { > > int fsize = ro->count * sizeof(struct can_filter); > > @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > > if (!err) > > err = put_user(len, optlen); > > return err; > > - > > + } > > case CAN_RAW_ERR_FILTER: > > if (len > sizeof(can_err_mask_t)) > > len = sizeof(can_err_mask_t); > > @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > > val = &ro->xl_frames; > > break; > > > > - case CAN_RAW_XL_VCID_OPTS: > > + case CAN_RAW_XL_VCID_OPTS: { > > + int err = 0; > > + > > /* user space buffer to small for VCID opts? */ > > if (len < sizeof(ro->raw_vcid_opts)) { > > /* return -ERANGE and needed space in optlen */ > > @@ -839,6 +842,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > > err = put_user(len, optlen); > > return err; > > > > + } > > case CAN_RAW_JOIN_FILTERS: > > if (len > sizeof(int)) > > len = sizeof(int); > > > > --- > > base-commit: c8fba5d6df5e476aa791db4f1f014dad2bb5e904 > > change-id: 20240220-raw-setsockopt-f6e173cdbbbb > > What is the target tree ? Sorry I have to fine tune my b4 setup, this targets net-next. And depends on https://lore.kernel.org/all/20240219200021.12113-1-socketcan@hartkopp.net/ > In net-next tree, syzbot complained about a bug added in > > commit c83c22ec1493c0b7cc77327bedbd387e295872b6 > Author: Oliver Hartkopp <socketcan@hartkopp.net> > Date: Mon Feb 12 22:35:50 2024 +0100 > > can: canxl: add virtual CAN network identifier support > > Patch to fix the issue has not been sent yet ? > > diff --git a/net/can/raw.c b/net/can/raw.c > index cb8e6f788af84ac65830399baac6d1cf3d093e08..897ffc17d850670003e5cf3402477e8fc201f61e > 100644 > --- a/net/can/raw.c > +++ b/net/can/raw.c > @@ -835,7 +835,9 @@ static int raw_getsockopt(struct socket *sock, int > level, int optname, > if (copy_to_user(optval, &ro->raw_vcid_opts, len)) > err = -EFAULT; > } > - break; > + if (!err) > + err = put_user(len, optlen); > + return err; > > case CAN_RAW_JOIN_FILTERS: > if (len > sizeof(int)) > The above mentioned patch fixes the issue introduced in c83c22ec1493 ("can: canxl: add virtual CAN network identifier support"). regards, Marc
On 20.02.2024 17:27:05, Vincent Mailhol wrote: > On Tue. 20 Feb. 2024 at 17:16, Marc Kleine-Budde <mkl@pengutronix.de> wrote: > > Reduce the scope of the variable "err" to the individual cases. This > > is to avoid the mistake of setting "err" in the mistaken belief that > > it will be evaluated later. > > > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > > One nitpick to remove a newline, but aside from that: > > Reviewed-by: Vincent Mailhol <mailhol.vincent@wanadoo.fr> > > (as usual, I do not mind if the nitpick gets resolved while applying). Thanks for the review, fixed that while applying. regards, Marc
On 20.02.2024 09:25:58, Eric Dumazet wrote: > In net-next tree, syzbot complained about a bug added in > > commit c83c22ec1493c0b7cc77327bedbd387e295872b6 > Author: Oliver Hartkopp <socketcan@hartkopp.net> > Date: Mon Feb 12 22:35:50 2024 +0100 > > can: canxl: add virtual CAN network identifier support > > Patch to fix the issue has not been sent yet ? Done, it's part of this PR: | https://lore.kernel.org/all/20240220085130.2936533-9-mkl@pengutronix.de/ regards, Marc
From: Marc Kleine-Budde > Sent: 20 February 2024 08:16 > > Reduce the scope of the variable "err" to the individual cases. This > is to avoid the mistake of setting "err" in the mistaken belief that > it will be evaluated later. > > Signed-off-by: Marc Kleine-Budde <mkl@pengutronix.de> > --- > net/can/raw.c | 12 ++++++++---- > 1 file changed, 8 insertions(+), 4 deletions(-) > > diff --git a/net/can/raw.c b/net/can/raw.c > index 897ffc17d850..2bb3eab98af0 100644 > --- a/net/can/raw.c > +++ b/net/can/raw.c > @@ -756,7 +756,6 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > struct raw_sock *ro = raw_sk(sk); > int len; > void *val; > - int err = 0; > > if (level != SOL_CAN_RAW) > return -EINVAL; > @@ -766,7 +765,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > return -EINVAL; > > switch (optname) { > - case CAN_RAW_FILTER: > + case CAN_RAW_FILTER: { > + int err = 0; > + > lock_sock(sk); > if (ro->count > 0) { > int fsize = ro->count * sizeof(struct can_filter); > @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > if (!err) > err = put_user(len, optlen); > return err; > - > + } > case CAN_RAW_ERR_FILTER: > if (len > sizeof(can_err_mask_t)) > len = sizeof(can_err_mask_t); > @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > val = &ro->xl_frames; > break; > > - case CAN_RAW_XL_VCID_OPTS: > + case CAN_RAW_XL_VCID_OPTS: { > + int err = 0; > + > /* user space buffer to small for VCID opts? */ > if (len < sizeof(ro->raw_vcid_opts)) { > /* return -ERANGE and needed space in optlen */ > @@ -839,6 +842,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, > err = put_user(len, optlen); > return err; > > + } > case CAN_RAW_JOIN_FILTERS: > if (len > sizeof(int)) > len = sizeof(int); I'd be very tempted to change the code so that there is only one put_user(len, optlen) right at the bottom. If the code is obeying the normal 'rules' for getsockopt() the function can actually return the length or an error and the caller can sort out what to do with the data. That makes the changes required to implement kernel_getsockopt() a lot simpler. I did start writing a patchset to do that, but some of the code is entirely brain-dead. But it would certainly make sense for the 'optlen' parameter to be a kernel address and have been validated to be 'sane' (eg positive) and have the syscall wrapper do the user copies. It can't be done for the buffer (in all cases) because the user supplied length isn't always the buffer length. But the length field is defined to be an integer. David - Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK Registration No: 1397386 (Wales)
diff --git a/net/can/raw.c b/net/can/raw.c index 897ffc17d850..2bb3eab98af0 100644 --- a/net/can/raw.c +++ b/net/can/raw.c @@ -756,7 +756,6 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, struct raw_sock *ro = raw_sk(sk); int len; void *val; - int err = 0; if (level != SOL_CAN_RAW) return -EINVAL; @@ -766,7 +765,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, return -EINVAL; switch (optname) { - case CAN_RAW_FILTER: + case CAN_RAW_FILTER: { + int err = 0; + lock_sock(sk); if (ro->count > 0) { int fsize = ro->count * sizeof(struct can_filter); @@ -791,7 +792,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, if (!err) err = put_user(len, optlen); return err; - + } case CAN_RAW_ERR_FILTER: if (len > sizeof(can_err_mask_t)) len = sizeof(can_err_mask_t); @@ -822,7 +823,9 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, val = &ro->xl_frames; break; - case CAN_RAW_XL_VCID_OPTS: + case CAN_RAW_XL_VCID_OPTS: { + int err = 0; + /* user space buffer to small for VCID opts? */ if (len < sizeof(ro->raw_vcid_opts)) { /* return -ERANGE and needed space in optlen */ @@ -839,6 +842,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname, err = put_user(len, optlen); return err; + } case CAN_RAW_JOIN_FILTERS: if (len > sizeof(int)) len = sizeof(int);