Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
From:   Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
To:     Simon Horman <horms@kernel.org>
CC:     open list <linux-kernel@vger.kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        Liam Howlett <liam.howlett@oracle.com>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "oliver.sang@intel.com" <oliver.sang@intel.com>
Subject: Re: [PATCH v2] Fix NULL pointer deref due to filtering on fork
Thread-Topic: [PATCH v2] Fix NULL pointer deref due to filtering on fork
Thread-Index: AQHZ/AGKCAsFOxKU5UCM3TNn1o0Z4LBHod+AgABHRIA=
Date:   Fri, 13 Oct 2023 16:16:20 +0000
Message-ID: <49E824D9-6D96-4FFA-99BD-A1301CE351C0@oracle.com>
References: <20231011051225.3674436-1-anjali.k.kulkarni@oracle.com>
 <20231013120105.GH29570@kernel.org>
In-Reply-To: <20231013120105.GH29570@kernel.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?R6hzNcpBhaUC6XuW/X9F8SPGZCb+vWPyAgMnf1gY9P0BqaG8qrP087YNH52N?=
 =?us-ascii?Q?8sg5MAwpv3nLoEjmSxeEauTXr14Z9OVZbLg6+UaM721UuqYNxxegFFRc368E?=
 =?us-ascii?Q?C4I2bkK0PDtjNXwEhMvls/RmkiJ2SxIZfxS17W6WR371gyexS4onH9XpyW7H?=
 =?us-ascii?Q?EUO+fQk51PDv4zozU3NNQrn/An0uY3e2XVEDg+jXyjVwnHbKDePMZHFaQPFN?=
 =?us-ascii?Q?KUVdPGhQBFGB9rjGIB9DU6x93HPNgXzyrUdASwmm4lmCzv3adI7dn8C5oOB2?=
 =?us-ascii?Q?1QVTgSjam9ccKkULG0PR8NfodaaDH78DCbZVowIuJjudHpOK7qK4gefPR2/X?=
 =?us-ascii?Q?vqhO9ezLyK4mmDsjKoH1k7ao6tx2Ny6pv/c1/61FV1hhzrVqyWRz9laN03pq?=
 =?us-ascii?Q?9MTaHyzGj/kvQJuTjzpxZO+GXIYbDc5emAoQGhpwDqkAGd6qS3I8wRBjB9k6?=
 =?us-ascii?Q?g0dMpZk+OViHxSeHeya27Rf/4vCgNoyIxdFpuc9Y8iEw0yZafEox4rWUzOp7?=
 =?us-ascii?Q?MzBYfKe+porG8nkGxQc9+lM3e1GORDuetUB7ECIgyMkbYlmV/KULmzHOWNRC?=
 =?us-ascii?Q?d7gxO1J0dCZJ0/miwjpoLpptl+sJJGfFkqFDWQTd6YOX4tX2nhlKzcLwe57f?=
 =?us-ascii?Q?JU8VdqMBz4WomC1KZT5ttnBowGVIEmHFhJwC+Qn6F175KDnIdG1ZgImPtC5W?=
 =?us-ascii?Q?CKdYvhpaS9XVE3RGDpd2t1/CY9NB9oWloyBMF0PfkHUXqTOs9tKAxquUxGp0?=
 =?us-ascii?Q?cs1yz/EcMRT6wkQsz/ISd/caUAxk8XP6MVo3lEq8YDot3HZZa5AFUjQ1ecMG?=
 =?us-ascii?Q?OflwNW7/GFYDW7aAf6x0i3pe8dHzugHb0vPux3qwRQpc0r5o3fKVcVb4PabN?=
 =?us-ascii?Q?zQBwMmZyTguqXR4pm3gRUMJw4YoX9JN/GQ9D8o1WYFkL//pTc8x+vBofwVMX?=
 =?us-ascii?Q?0rNutH6D2nl/uScOKfpMnQMssr3wia9j8A9dAQXM0zqzmRgVg4tf5sTzFuf/?=
 =?us-ascii?Q?IOL4QNdOZ+QdqMF2OlJc50/yHmrrOoSa18sF0dTS0aXKa/wZL0InKTE6wiHj?=
 =?us-ascii?Q?8W28J98c1ikvK+xCcVvXy2G+C6VtQuJtLF9fc0uGZP4hWELHxQHoQ3C1L48D?=
 =?us-ascii?Q?uYlaGzymLgE5gPmyWbOGs17PVW+ExUz7PXrUfhKu2neFvfwYBTTXStlWB42m?=
 =?us-ascii?Q?VyPvb6W6qcZdlA+EpXjw/lMYnPl5NPbVvXh/X74TSNJTI6SQ3sSeTN6y1x6A?=
 =?us-ascii?Q?rufb6g6+N056gf9+Yl10Pd7Uy19dB0Qztyxd5GwxnAcwJ1d/ufBwjHnetSoI?=
 =?us-ascii?Q?Gy0y8Rh/Buox9d60jV+08x4NjbOx00IyjqnzKi4+Ur0wla1DKEEdEdebNA51?=
 =?us-ascii?Q?KHWkW2835pUxFKqsHcnEf5hVoZHZ/A11NwUBo47WTwy08cGsD/PPHqJZ+5Xr?=
 =?us-ascii?Q?iy4cic52p94p6FtYyn6SObRBiCjdzoMTtnfliuDbQc0VnwjIIlie41e71BuR?=
 =?us-ascii?Q?XEZRmxCOIs8yofWJ0PpHZfs7IzdN1Q+iEg1VsOGbQw335k60S6H3nshXgOv0?=
 =?us-ascii?Q?QlUyZfdMo3oZK/6bRBgksZekBCGST8WlgSy2GI3fmBuD5UJ4jQ2Fu5T5r8/k?=
 =?us-ascii?Q?cw=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <C1A5BDD87CEA1C48AA1CCCB30E915653@namprd10.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: xWuqVbXqlFbpB4++pI/0USFo+AvrcYTXGQwBsAkD0S4/rMgn9FFWhsw96veFrrs/SKM7PfWnPZT4jQhN+Ntz6cplw6V2r7QwSLvmow5654trVlgw+P2Nne+cemoBJoabUh3xPnweX0sKcH9UWkU6Qb88uccB/YM4eRNIEZBhaDm55dcExEbvhw2zyX/eG5HW94YwS6teFDwqdo9wqJNcGB/HeUUqCniEFx5E+lC/cLB4K9kAcZPfJkcF+ZAZYJzTEQ4OQVwDbxUYHtmGBCTACJkPOoHevw/zK4hLa7W6p0TzIqfwEScHfGm0BuC1cl7gixOJQYQMd2bVcXBur1yADQCfMSr3eYZvZ/nN9yMmiRhah6Xyi7MhdW0mKo3bTUn9DCdG82mf49n2So0I3qcf08V5G8NnLf56Lr0dvEcG+rIlxH/6yXV54zO9n7HrRkrbTxPM8AmGAD7NQBIInzMNzea95vMW1BZSjYu92Bwphjfj6mwBB5GERDJhoQB/Detbr0QfRBT/zgxpzMn6UkbP2lJrlx0Moe/5gnRTTO406lX0QWxnL5BkGbPiHzaLxoVNG+IOdudC9/dSotHbYg89NOHHinZTX5N9QsK0uqnJ1kAnuj0+boAuCAhIyGUPqeT8nri8pz4GtD2X0sH8VSfpfcEidgfdob+Pe79YnOXX+O9pwz/tXlhnkU5mQTLXO8Ui7THnu8DEknsdA25y1/kReIsBrPO8pkzHkHCiVwCCGeJczoVHzKjzEwsQ6g5/tyLcNb846Kgr3LTnHkJyu0HhzXbQYTBnrn5oUO0nUjCO7XO2Q3tO0tk527TGzwn4c21K0dux/2vt2Dm2/o6lJqXAd3BUTenHVyB3/1s/7SLdyW8=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4129.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 29967061-1bc5-4a7a-11b3-08dbcc07bc7b
X-MS-Exchange-CrossTenant-originalarrivaltime: 13 Oct 2023 16:16:20.1902
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: BoLy0uGBBd/Zcpgfc4Oxil5CQvJiBWLV+7ZMGDNYLlhG6SQjbWJgWHOlj81hatT4jGc0p0xiEHwSOaZkVUZDYFxgb+9gd0pM/y3g4ndDeuU=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: PH7PR10MB6628
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.980,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2023-10-13_07,2023-10-12_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 malwarescore=0 adultscore=0
 suspectscore=0 phishscore=0 spamscore=0 mlxscore=0 mlxlogscore=999
 bulkscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2309180000 definitions=main-2310130138
X-Proofpoint-ORIG-GUID: XaleLkES1xdvFAqhr3w7Fq8OEQ_askcS
X-Proofpoint-GUID: XaleLkES1xdvFAqhr3w7Fq8OEQ_askcS
X-Spam-Status: No, score=-0.8 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Fri, 13 Oct 2023 09:19:20 -0700 (PDT)


> On Oct 13, 2023, at 5:01 AM, Simon Horman <horms@kernel.org> wrote:
>=20
> On Tue, Oct 10, 2023 at 10:12:25PM -0700, Anjali Kulkarni wrote:
>> cn_netlink_send_mult() should be called with filter & filter_data only
>> for EXIT case. For all other events, filter & filter_data should be
>> NULL.
>>=20
>> Fixes: 2aa1f7a1f47c ("connector/cn_proc: Add filtering to fix some bugs"=
)
>> Reported-by: kernel test robot <oliver.sang@intel.com>
>> Closes: https://urldefense.com/v3/__https://lore.kernel.org/oe-lkp/20230=
9201456.84c19e27-oliver.sang@intel.com__;!!ACWV5N9M2RV99hQ!PgqlHq_nOe_KlyKk=
B9Mm_S8QstTJvicjuENwskatuuQK05KPuFw-KvRZeOH8iuEAMjRhkxEMPKJJnLcaT8zrPf9aqNs=
$
>=20
> For the record, this got a bit mangled. I believe it should be:
>=20
> Closes: https://lore.kernel.org/oe-lkp/202309201456.84c19e27-oliver.sang@=
intel.com/
>=20
> Also, there is probably no need to resend because of this,
> but no blank line here, please.

Ok.

>=20
>> Signed-off-by: Anjali Kulkarni <anjali.k.kulkarni@oracle.com>
>> ---
>> drivers/connector/cn_proc.c | 8 ++++----
>> 1 file changed, 4 insertions(+), 4 deletions(-)
>>=20
>> diff --git a/drivers/connector/cn_proc.c b/drivers/connector/cn_proc.c
>> index 05d562e9c8b1..01e17f18d187 100644
>> --- a/drivers/connector/cn_proc.c
>> +++ b/drivers/connector/cn_proc.c
>> @@ -104,13 +104,13 @@ static inline void send_msg(struct cn_msg *msg)
>> if (filter_data[0] =3D=3D PROC_EVENT_EXIT) {
>> filter_data[1] =3D
>> ((struct proc_event *)msg->data)->event_data.exit.exit_code;
>> + cn_netlink_send_mult(msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
>> +      cn_filter, (void *)filter_data);
>> } else {
>> - filter_data[1] =3D 0;
>> + cn_netlink_send_mult(msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
>> +      NULL, NULL);
>> }
>>=20
>> - cn_netlink_send_mult(msg, msg->len, 0, CN_IDX_PROC, GFP_NOWAIT,
>> -      cn_filter, (void *)filter_data);
>> -
>=20
> I am wondering if you considered making cn_filter slightly smarter.
> It seems it already understands not to do very much for PROC_EVENT_ALL.

I could, but then we would be unnecessarily make an extra function call to =
cn_filter, and re-check this condition, which can be avoided if we do the c=
heck here.=20

>=20
>> local_unlock(&local_event.lock);
>> }
>>=20
>> --=20
>> 2.42.0