Date:   Wed, 5 Jan 2022 10:37:05 -0400
From:   Jason Gunthorpe <jgg@nvidia.com>
To:     trondmy@kernel.org
Cc:     linux-rdma@vger.kernel.org, linux-nfs@vger.kernel.org
Subject: Re: [PATCH] RDMA: null pointer in __ib_umem_release causes kernel
 panic
Message-ID: <20220105143705.GS2328285@nvidia.com>
References: <20220105141841.411197-1-trondmy@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220105141841.411197-1-trondmy@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?yd2uxWD2qvi3jMA0mVlmMT/fOGz2k71dJIn2NaezTuOCfdBsKI9rvRr1OSSa?=
 =?us-ascii?Q?yVgOAMTqZFFO8c+ASMvuNtetOuyEPc2GGzVN3Wm9lIsu1z6GhMjV15nuJKer?=
 =?us-ascii?Q?D6uWYsST9wtMxf/9sTeI7vY8+tdBribDZsAIeF1PebOUZP/b6dEMfWW2mN0G?=
 =?us-ascii?Q?qBit1kmo2G8RJHF7ZHXy5tw2NkVJFdALdWhPOJzAX3jcTQ3qffQMBbrB8GiX?=
 =?us-ascii?Q?BEpnTlUIm6fPDFW+N7jHJdpMRLntuXi7F3rGoF+J+JJzDvW5apgFLdMdxUjp?=
 =?us-ascii?Q?1Er8GvXji2gwSnX6rY7iN34Ps7SH0UpirfjoSgyA9F4BubfbaM6XIB/54qJm?=
 =?us-ascii?Q?RR9N09y1Eo6tYHeSBQwhHcMTcPYmGRnPZoCANLOyqGKZs1wK5BBAz4o9srBl?=
 =?us-ascii?Q?KcKRjZqbhS/A/uhioOUuvJKMDQL5TVy5Rz+GOPFFl+4S27SygA1rW6rwOZV8?=
 =?us-ascii?Q?l3Q52quVrvUwSJKDr6J2UJvup9PZLaTUvPTBSb+JQrPHTznMNmvY23Hvt8ds?=
 =?us-ascii?Q?hSGUO2qy46x1yd2aXW42C3PG4Hp82jIILjOPTtZLOwFnUkWYDgsdkPYL3GkK?=
 =?us-ascii?Q?6njfIrwvb7znCt53rtLCuqsXorAMo8DFJcC9VdL9xiDjM5prEQde3LoEIP5U?=
 =?us-ascii?Q?/fCobL6RNuwiBNt9Tpnf7YKY+776y2FljJ848QbwtJslmwm+CADL1yUaKgSm?=
 =?us-ascii?Q?irWV19A3GRV9t2LpIV9KIC6D4AT/Zq6Re4iyL/DiXz5eMzTcHn8fy4Oo38qB?=
 =?us-ascii?Q?3MG6hHnjAMnJFpM9fw+dzbkzxErv0RYG9g8SHgBjd6gDr1OPLG34NU9zeIRt?=
 =?us-ascii?Q?ZmWpDFGn5UC4oEi20pEnje1Pm2UvXQ8ONhDddHdRfQYpaYm8Fpt7pNIVUrcQ?=
 =?us-ascii?Q?UIMvETTk+ddnIlAwqMvgGRTal0FZm4wMKF0i0ZpESUcbocDII+Gp26bJCKzW?=
 =?us-ascii?Q?739ATm5YmXzISjev1OU+ajo3GSd4p0qOacZBvtR39IF9RNfCWHiG2vZPnCIT?=
 =?us-ascii?Q?sKK+5St+rmJIXaTjTbCWqb3BVdoBcjKG/DJmC252/VYwg0B3an2tAQOnGIrd?=
 =?us-ascii?Q?0HGpwJsbBocbuQetcrs6yG3owd2Ubf4KbRJh7wdo6tdPbj2aePARW9JEWzxe?=
 =?us-ascii?Q?zgUsPzTwxC8LmMGLc7ebOPbz0jMJTfLLJW+ltPo+hj4KHDVrgS29kOh29m+k?=
 =?us-ascii?Q?tWgXVKI+tQvxleUPkULg0rjZhO6pAdDYbEggHOPZBManyeQuGo4xDzeChQ1S?=
 =?us-ascii?Q?j0jJapJNcgArqRNqxK/wdGsG+1syljhlEwZb0PBUpmsO8lPiC+spR3nrKqqM?=
 =?us-ascii?Q?vfC1li8Pm0b3jqkaJCkIqa4mnRlEBHbhwnP6wOKXSRxB9jwMLnOA/GmgrhJE?=
 =?us-ascii?Q?awpveF9sm19POI0F7FsXB4qucAI5FaQ/ef+EGWvTeZIzy3VQoT00zuY6f9/G?=
 =?us-ascii?Q?QjJPXEAQ1zdZseOznqkhRS3ttSJSUetSOH9UzEI0OhvHYj+yawzLYkfNFjNv?=
 =?us-ascii?Q?/gk4HioCT0eJkOJJd/Iz72H8olaKfAia70opRakpYnXlVCAchu4FaVg7yWjo?=
 =?us-ascii?Q?6riRFSvMgQw3yWxop3I=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: adeec3b3-addd-4dfe-5d2d-08d9d058d8f4
X-MS-Exchange-CrossTenant-AuthSource: BL0PR12MB5506.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jan 2022 14:37:06.7103
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: TYzhPtd9pfinxa0DXjpFyixyd14npuB42TPNlDMS2kWWSjVapu26du4WBVtTyxJm
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5110
Precedence: bulk
List-ID: <linux-nfs.vger.kernel.org>
X-Mailing-List: linux-nfs@vger.kernel.org

On Wed, Jan 05, 2022 at 09:18:41AM -0500, trondmy@kernel.org wrote:
> From: Trond Myklebust <trond.myklebust@hammerspace.com>
> 
> When doing RPC/RDMA, we're seeing a kernel panic when __ib_umem_release()
> iterates over the scatter gather list and hits NULL pages.
> 
> It turns out that commit 79fbd3e1241c ended up changing the iteration
> from being over only the mapped entries to being over the original list
> size.

You mean this?

-       for_each_sg(umem->sg_head.sgl, sg, umem->sg_nents, i)
+       for_each_sgtable_sg(&umem->sgt_append.sgt, sg, i)

I don't see what changed there? The invarient should be that

  umem->sg_nents == sgt->orig_nents

> @@ -55,7 +55,7 @@ static void __ib_umem_release(struct ib_device *dev, struct ib_umem *umem, int d
>  		ib_dma_unmap_sgtable_attrs(dev, &umem->sgt_append.sgt,
>  					   DMA_BIDIRECTIONAL, 0);
>  
> -	for_each_sgtable_sg(&umem->sgt_append.sgt, sg, i)
> +	for_each_sgtable_dma_sg(&umem->sgt_append.sgt, sg, i)
>  		unpin_user_page_range_dirty_lock(sg_page(sg),

Calling sg_page() from under a dma_sg iterator is unconditionally
wrong..

More likely your case is something has gone wrong when the sgtable was
created and it has the wrong value in orig_nents..

Jason