Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Colin Foster <colin.foster@in-advantage.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Eric Dumazet <edumazet@google.com>,
        "David S. Miller" <davem@davemloft.net>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>
Subject: Re: [PATCH v1 net 2/2] net: mscc: ocelot: fix possible memory
 conflict for vcap_props
Thread-Topic: [PATCH v1 net 2/2] net: mscc: ocelot: fix possible memory
 conflict for vcap_props
Thread-Index: AQHYXCE0wfZ6v7cjxEOPYespSpyp0K0Ig9+A
Date:   Sat, 30 Apr 2022 14:24:57 +0000
Message-ID: <20220430142457.7l2towhbptdvrfje@skbuf>
References: <20220429233049.3726791-1-colin.foster@in-advantage.com>
 <20220429233049.3726791-3-colin.foster@in-advantage.com>
In-Reply-To: <20220429233049.3726791-3-colin.foster@in-advantage.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?9+rcy0ohodJYiTd7nCltJ7f48/ZkSowtE6t70YNbmzBPtWqUf/dnrERXiiB6?=
 =?us-ascii?Q?NAq1hdM18E1o83cUNvhKxuqs9zRcb53apo8D+6JZBSwtQJkjCpq4meyByy1X?=
 =?us-ascii?Q?jiAd7tBHUocwTiEMQKA2pVChvAHCVc5SpkViy3E9QRwvvqBSMmAuePPZVz/n?=
 =?us-ascii?Q?A+a/4aSGzXbSxe4C1AF7P+wf+EVCxCfYh3YsQCPHEEEycAq4GL8UGc0mCoMZ?=
 =?us-ascii?Q?7iC7iKkb4mNdsciRgjs7CE3HYeAtVxfyQN/8C9R2PvYI49wTm5ZmOuHdHz4v?=
 =?us-ascii?Q?f+gnHmtpLNKJfCM80HEe50nR9Eisl4QY8FzQjoUirkIGnCh301hboRIfY2Xl?=
 =?us-ascii?Q?oM9oE+geAurVbUJkymb0U9K7zlKYcmT7/egp85UT9v5frrUxfbyWGpDOtOZm?=
 =?us-ascii?Q?ZnG2PiF3F90ijgVVYVjMKreGlZAZq18uLmbmAm5g7Pm/hr+B9AFVDkXeMfLq?=
 =?us-ascii?Q?H4U8rxRdGhMOw4fK2e8l5o/OnAtTA34IcoxSKdbS4ROzExafIejNMmpnNVW0?=
 =?us-ascii?Q?aNr/w/bZ8+SE6ppoAHia8oMouj8syMP7YkfWEkhXj+lS/95P46PVSnGtoZ6+?=
 =?us-ascii?Q?Zm0HuYDa7QYWsSQb6pKd1pvnL13cuWl+LnAPn9Rr+3Z9ch4cKnWxTrnL9TSn?=
 =?us-ascii?Q?d5u78bcDJgvKyQNcJv5H7JjTS7Kdvf8xmeiDziigN9M8fezTzqnvfGeX8M2b?=
 =?us-ascii?Q?+P4qRrH5Q/WrEllaeQYYQU6z60Zw6LcrrbbRRPtuUPfuA8Jk6f80rdUIMf7V?=
 =?us-ascii?Q?EnHZlUyqHaORrYLsufnnoJBQubQpPtVsam06SzDJWaKNAyHWyQuDFCRgoosV?=
 =?us-ascii?Q?R1tonODxIbW03AWGUlrtgZfouZSchhzEFWRNHvcwQyq/3RJoV5HDFnTh1GVv?=
 =?us-ascii?Q?+hmyA1WuU9aoBY57g+DW14ct7BCWqqT+zL/7W93KDAzPFJDwyBQ5J8MeosUv?=
 =?us-ascii?Q?+tLTgcDiSRTCT5wd8IB1IpLEC1V9VrteQLWQtOs/WrEb1rGCi6KvDMce+25t?=
 =?us-ascii?Q?xAWofMn09P5yO2PQW265pGqYGkN/kxpDEj+MXbZzZz11tqkVgdsVi879GfuE?=
 =?us-ascii?Q?TAzmuvH9LCvjzpCnZBFE3ttUOSW6Pbl8NVLvGohz3ie7TxBx0UrtBC4y3Quj?=
 =?us-ascii?Q?o1kYZH9qHkcVcEu7R/G25JFqqg55to87AMsqObq2Mu/I8vyeCWswBD44VumF?=
 =?us-ascii?Q?5gQNLuCIZr9d/SQvqcMEXfgpaYsyVb69NMJCf3n85Ghx188GSJMoVHWzamBm?=
 =?us-ascii?Q?9MvjQRSwcStaQp/zL5QfSMYCqnvHuRAqc8bB9+B8LT0Qz3XsXy6mSPfVPX5X?=
 =?us-ascii?Q?2Aeox87SRjMMNGIMFwawNRF5eefN3FcCob7jkakhTPcoFnBJM1aChs3u+F+x?=
 =?us-ascii?Q?n0fbk4SRnIK7KmqBB3zmJu2yt41XFtvb39V6nrKuF4PChwNTl7M1hZyGvxWJ?=
 =?us-ascii?Q?sWxfqHK+iGR3OZaXdgOVWrhmes8O6i7Cdxb15VhuiNsQ3j9H8VyIInzs9RMJ?=
 =?us-ascii?Q?nw6wvcqkyYeequGIyQi0ez8ViLxBWvKrM56+cSaTsRWekjKJbrbKhx0UiMmy?=
 =?us-ascii?Q?IoCjwm8iR18h2wthhSlbKcoSzDb/dT/AdNpT+6vvJyvSruk+q8B9GMAWIreV?=
 =?us-ascii?Q?Zx5CE0hviOeX2RbUeMzdSGtNtJC63nJufxOryphjFXCP2AvP05Eb8etKLR6R?=
 =?us-ascii?Q?/SdUa/tUXfcZc6OuF8uthirG1sjy409D7VDv8k4IfXkbZ3UUTrnSZXysTzCO?=
 =?us-ascii?Q?Q9W/2xTDn7T7QJAjupNc25icWmkxXgE=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3C835CC709FB7C4F8DE6E99A20C6A9BC@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 0966b035-da87-4e5b-f282-08da2ab53425
X-MS-Exchange-CrossTenant-originalarrivaltime: 30 Apr 2022 14:24:57.8383
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 5MwgCYK5ugkCWOK80dTcoVMcxMEnooMupRAIhsPgVuMTCvHfxswTlWZqBu7JRpa6ZvHMlK+pwVgtFDApQ8jZzQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB6205
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS autolearn=ham
        autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Colin,

On Fri, Apr 29, 2022 at 04:30:49PM -0700, Colin Foster wrote:
> Each instance of an ocelot struct has the ocelot_vcap_props structure bei=
ng
> referenced. During initialization (ocelot_init), these vcap_props are
> detected and the structure contents are modified.
>=20
> In the case of the standard ocelot driver, there will probably only be on=
e
> instance of struct ocelot, since it is part of the chip.
>=20
> For the Felix driver, there could be multiple instances of struct ocelot.
> In that scenario, the second time ocelot_init would get called, it would
> corrupt what had been done in the first call because they both reference
> *ocelot->vcap. Both of these instances were assigned the same memory
> location.
>=20
> Move this vcap_props memory to within struct ocelot, so that each instanc=
e
> can modify the structure to their heart's content without corrupting othe=
r
> instances.
>=20
> Fixes: 2096805497e2b ("net: mscc: ocelot: automatically detect VCAP
> constants")
>=20
> Signed-off-by: Colin Foster <colin.foster@in-advantage.com>
> ---

To prove an issue, you must come with an example of two switches which
share the same struct vcap_props, but contain different VCAP constants
in the hardware registers. Otherwise, what you call "corruption" is just
"overwriting with the same values".

I would say that by definition, if two such switches have different VCAP
constants, they have different vcap_props structures, and if they have
the same vcap_props structure, they have the same VCAP constants.

Therefore, even in a multi-switch environment, a second call to
ocelot_vcap_detect_constants() would overwrite the vcap->entry_width,
vcap->tg_width, vcap->sw_count, vcap->entry_count, vcap->action_count,
vcap->action_width, vcap->counter_words, vcap->counter_width with the
exact same values.

I do not see the point in duplicating struct vcap_props per ocelot
instance.

I assume you are noticing some problems with VSC7512? What are they?
Note that since VSC7512 isn't currently supported by the kernel, even a
theoretical corruption issue doesn't qualify as a bug, since there is no
way to reproduce it. All the Microchip switches supported by the kernel
are internal to an SoC, are single switches, and they have different
vcap_props structures.=