From mboxrd@z Thu Jan  1 00:00:00 1970
From: Claude Code Review Bot <claude-review@example.com>
To: dri-devel-reviews@example.com
Subject: Claude review: relay: zero page->private when freeing pages
Date: Tue, 24 Feb 2026 10:52:52 +1000
Message-ID: <review-patch1-20260223032641.1859381-2-ziy@nvidia.com>
In-Reply-To: <20260223032641.1859381-2-ziy@nvidia.com>
References: <20260223032641.1859381-1-ziy@nvidia.com> <20260223032641.1859381-2-ziy@nvidia.com>
X-Mailer: Claude Code Patch Reviewer
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0

Patch Review

There is a bug in the depopulate error path. The patch uses `i` when it should use `j`:

> depopulate:
> -	for (j = 0; j < i; j++)
> +	for (j = 0; j < i; j++) {
> +		set_page_private(buf->page_array[i], 0);
> 		__free_page(buf->page_array[j]);
> +	}

The `set_page_private` call uses `buf->page_array[i]`, but at the `depopulate` label, `i` is the index where `alloc_page()` failed. That means `buf->page_array[i]` is NULL (the allocation returned NULL). This will cause a NULL pointer dereference in `set_page_private`. The correct index is `j`, matching the `__free_page` call on the next line.

The second hunk in `relay_destroy_buf` correctly uses `i` for both calls since the same loop variable is used throughout:

> -		for (i = 0; i < buf->page_count; i++)
> +		for (i = 0; i < buf->page_count; i++) {
> +			set_page_private(buf->page_array[i], 0);
> 			__free_page(buf->page_array[i]);
> +		}

This part is correct.

---
Generated by Claude Code Patch Reviewer