Date:   Thu, 2 Feb 2023 10:05:35 +0100
From:   Simon Horman <simon.horman@corigine.com>
To:     Eelco Chaudron <echaudro@redhat.com>
Cc:     Fedor Pchelkin <pchelkin@ispras.ru>,
        Pravin B Shelar <pshelar@ovn.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>, netdev@vger.kernel.org,
        dev@openvswitch.org, linux-kernel@vger.kernel.org,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        lvc-project@linuxtesting.org
Subject: Re: [PATCH v2] net: openvswitch: fix flow memory leak in
 ovs_flow_cmd_new
Message-ID: <Y9t832LOl5FsC3dv@corigine.com>
References: <20230201210218.361970-1-pchelkin@ispras.ru>
 <4331E34B-4237-48D0-B4E0-016E45176FD1@redhat.com>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <4331E34B-4237-48D0-B4E0-016E45176FD1@redhat.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?tFgo8dgjBeYSpMmXdhDPSaMWeWohFWSbILJGeAVpbISMFwL9UtSO6MIDKHUs?=
 =?us-ascii?Q?CuAdV8n8+EmkIE5mDu2/qpGRHhcoqeuN6a2Ve1uirKfYrfV8f1P793bNtkhS?=
 =?us-ascii?Q?i5CWYIswfp6U6MS1tAF3RDE4xPe88MCEYknBKsteDS4jiLhcuZ657rYH8XKV?=
 =?us-ascii?Q?i4t87cECCGEAvKmUNEA3jMsXmgYIFRQuKxHYoxsz3Nwl/1ExdZQrd1I8xlqe?=
 =?us-ascii?Q?pKNeq2xoUGuS3INqpwfCZpZ9x92+D3F43jV/MBia43PR8/ifKTdyZeDJ4fTx?=
 =?us-ascii?Q?CeajspNjHzE/49wPiDkR2pFDcqjKPPyPJsjdHH1FTnmXq7NQRpgMh+8MEnAb?=
 =?us-ascii?Q?im0+oDzQjuUSWyzThSqdLbv9NDZcGtMyt20hyqXIaZc4seb/kn4LZmDkrpY1?=
 =?us-ascii?Q?3EJRpE1tAB3QwVXYcHhpbwp1UUfy8YQQhPI1kvKyGb0VKYr2warBf9X7nfmF?=
 =?us-ascii?Q?midZ3eCSPOp7mJN79ykGCT1+GrxWxolMbwfPQjIp3uJUvog57BncZwdA4ezN?=
 =?us-ascii?Q?pJdzsVmJjsX8B/cF6esMOpJvfYZGKYj9ImLlKY/7RbhX84Zt3pwAsra49SwS?=
 =?us-ascii?Q?JuOppEw989I3Un9Pkcf35EOtxFQca2fe79Ru56GWTDcO1Sx3dp5YRwhBqO7E?=
 =?us-ascii?Q?HcCFmAoxb9s3xQ1kUFuPUBRc+vk6M8NFXkC5niliI4T43v/OYLvZjwhC1UNH?=
 =?us-ascii?Q?/b+gjFvSiwOjsxBIt3rbmf2DKo6KGxYe66kq8F9dQFtLljEWXcmpPk3e+gSK?=
 =?us-ascii?Q?Ru2NAwVsaaSpGIg63Xgxr9D1CtTaS/kqR9pwWsJurwNmScH90npsEihGMFnI?=
 =?us-ascii?Q?evOl/2xmU3VOZfi8TKYL9r8nlQ0q9Yl0m85jURZMcizE93c8gZzpTNs75YDO?=
 =?us-ascii?Q?aP9inKxRk19b73oRNbOeGam2KWYPArUDidH/oL5xgPtQth4qhafYHC1xqz/4?=
 =?us-ascii?Q?7PwtNYWcZA0023xw8SWhvcBEGa10pa5rTi8vpAXt7+Jff6Mr1EFGt8ArYfof?=
 =?us-ascii?Q?wqGwIVgTXLK7IXIA0ZmFuCaMWA+bPnpHCs71tQb0GxvYSOZEUuYqsDG8cG1P?=
 =?us-ascii?Q?Ra5HKIaWOanWemZD9QpxpF/yJogPErd8giTBsLBtsex5U3XJvPPbL8pedCZ0?=
 =?us-ascii?Q?/iT/Umv3sozRNSL2rth8LtbZHd5fEDCVK6k4vcxtx760znzakila9qvpjPMw?=
 =?us-ascii?Q?nHhILLghDw0aQQATb84pZApmFMQamA9OWTDXuxYW0W1LSCW58vSe/0uFGnZE?=
 =?us-ascii?Q?BYR7N9DytQCWT2cqDu+sdSiQ9M8Ak60qMnO4pw89YQrp31aVVQujJyapl2sF?=
 =?us-ascii?Q?Mf0L7KPk6A8pnDz3HL/E4JQQ2dAVeO2ebmAscOfpjERtfxd/o8Plb0qWkhIc?=
 =?us-ascii?Q?El7k2mrr7cZ8wqECrLaUTBc9VsgdsVYWbldI3t2aj3E5ptaXmMj4FJcdBcJU?=
 =?us-ascii?Q?ApSPGvmct7dECLHC+7KEXib9LdB6mTpFzWQjP95Z7D98XFc3eKTmjGerXdbH?=
 =?us-ascii?Q?MyiNFhPCWjBn34Wi5zGqzz5/qHSwNcdYnMFJPct+6ScRsLtnrJ3o1+VSgmOi?=
 =?us-ascii?Q?4m1FZ/EjfWsb63mKx+JTIadYvSQh1EowPDoA6pRJSG6TZRFVHU6PVAcc6XnU?=
 =?us-ascii?Q?AoGJFKcWX7eqBbAkEtID2x5QIamgtv0wu7MizTAHkJkvHaS3dXTeQz/gi8vP?=
 =?us-ascii?Q?MWcrCQ=3D=3D?=
X-OriginatorOrg: corigine.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 6619315f-e357-4afe-6564-08db04fca923
X-MS-Exchange-CrossTenant-AuthSource: BY3PR13MB4834.namprd13.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 02 Feb 2023 09:05:42.0714
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: fe128f2c-073b-4c20-818e-7246a585940c
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: xme6LD638rHjlpULhFjPqSL6uUuo8yathIK+OCPhQ0Le086qbAPZG5pjqkPyLqvuqTxcZFpg0QrLHdoNlU0LTJn9JXQ7DX+CvfooZRQuGZs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR13MB4417
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Thu, Feb 02, 2023 at 09:11:04AM +0100, Eelco Chaudron wrote:
> 
> 
> On 1 Feb 2023, at 22:02, Fedor Pchelkin wrote:
> 
> > Syzkaller reports a memory leak of new_flow in ovs_flow_cmd_new() as it is
> > not freed when an allocation of a key fails.
> >
> > BUG: memory leak
> > unreferenced object 0xffff888116668000 (size 632):
> >   comm "syz-executor231", pid 1090, jiffies 4294844701 (age 18.871s)
> >   hex dump (first 32 bytes):
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00  ................
> >   backtrace:
> >     [<00000000defa3494>] kmem_cache_zalloc include/linux/slab.h:654 [inline]
> >     [<00000000defa3494>] ovs_flow_alloc+0x19/0x180 net/openvswitch/flow_table.c:77
> >     [<00000000c67d8873>] ovs_flow_cmd_new+0x1de/0xd40 net/openvswitch/datapath.c:957
> >     [<0000000010a539a8>] genl_family_rcv_msg_doit+0x22d/0x330 net/netlink/genetlink.c:739
> >     [<00000000dff3302d>] genl_family_rcv_msg net/netlink/genetlink.c:783 [inline]
> >     [<00000000dff3302d>] genl_rcv_msg+0x328/0x590 net/netlink/genetlink.c:800
> >     [<000000000286dd87>] netlink_rcv_skb+0x153/0x430 net/netlink/af_netlink.c:2515
> >     [<0000000061fed410>] genl_rcv+0x24/0x40 net/netlink/genetlink.c:811
> >     [<000000009dc0f111>] netlink_unicast_kernel net/netlink/af_netlink.c:1313 [inline]
> >     [<000000009dc0f111>] netlink_unicast+0x545/0x7f0 net/netlink/af_netlink.c:1339
> >     [<000000004a5ee816>] netlink_sendmsg+0x8e7/0xde0 net/netlink/af_netlink.c:1934
> >     [<00000000482b476f>] sock_sendmsg_nosec net/socket.c:651 [inline]
> >     [<00000000482b476f>] sock_sendmsg+0x152/0x190 net/socket.c:671
> >     [<00000000698574ba>] ____sys_sendmsg+0x70a/0x870 net/socket.c:2356
> >     [<00000000d28d9e11>] ___sys_sendmsg+0xf3/0x170 net/socket.c:2410
> >     [<0000000083ba9120>] __sys_sendmsg+0xe5/0x1b0 net/socket.c:2439
> >     [<00000000c00628f8>] do_syscall_64+0x30/0x40 arch/x86/entry/common.c:46
> >     [<000000004abfdcf4>] entry_SYSCALL_64_after_hwframe+0x61/0xc6
> >
> > To fix this the patch rearranges the goto labels to reflect the order of
> > object allocations and adds appropriate goto statements on the error
> > paths.
> >
> > Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> >
> > Fixes: 68bb10101e6b ("openvswitch: Fix flow lookup to use unmasked key")
> > Signed-off-by: Fedor Pchelkin <pchelkin@ispras.ru>
> > Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> > ---
> > v1->v2: make goto statements structured
> 
> Thanks for fixing this, the changes look good.
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>

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