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;
Content-Type: text/plain;
        charset=us-ascii
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3696.80.82.1.1\))
Subject: Re: [PATCH] fs/proc/kcore.c: remove check of list iterator against
 head past the loop body
From:   Jakob Koschel <jakobkoschel@gmail.com>
In-Reply-To: <20220331164843.b531fbf00d6e7afd6cdfe113@linux-foundation.org>
Date:   Fri, 1 Apr 2022 09:19:57 +0200
Cc:     Mike Rapoport <rppt@kernel.org>,
        David Hildenbrand <david@redhat.com>,
        Oscar Salvador <osalvador@suse.de>,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        Brian Johannesmeyer <bjohannesmeyer@gmail.com>,
        Cristiano Giuffrida <c.giuffrida@vu.nl>,
        "Bos, H.J." <h.j.bos@vu.nl>
Content-Transfer-Encoding: quoted-printable
Message-Id: <A23914B0-BFD7-48D6-ADCF-42062E1D9887@gmail.com>
References: <20220331223700.902556-1-jakobkoschel@gmail.com>
 <20220331164843.b531fbf00d6e7afd6cdfe113@linux-foundation.org>
To:     Andrew Morton <akpm@linux-foundation.org>
Precedence: bulk


> On 1. Apr 2022, at 01:48, Andrew Morton <akpm@linux-foundation.org> =
wrote:
>=20
> On Fri,  1 Apr 2022 00:37:00 +0200 Jakob Koschel =
<jakobkoschel@gmail.com> wrote:
>=20
>> When list_for_each_entry() completes the iteration over the whole =
list
>> without breaking the loop, the iterator value will be a bogus pointer
>> computed based on the head element.
>>=20
>> While it is safe to use the pointer to determine if it was computed
>> based on the head element, either with list_entry_is_head() or
>> &pos->member =3D=3D head, using the iterator variable after the loop =
should
>> be avoided.
>>=20
>> In preparation to limit the scope of a list iterator to the list
>> traversal loop, use a dedicated pointer to point to the found element =
[1].
>>=20
>> ...
>>=20
>=20
> Speaking of limiting scope...

Fair point :-)

I see you have applied this already to the -mm tree. Shall I still move =
the iterator?
The hope is to remove the 'iter' variable altogether when there are no =
uses after
the loop anymore.

>=20
> --- =
a/fs/proc/kcore.c~fs-proc-kcorec-remove-check-of-list-iterator-against-hea=
d-past-the-loop-body-fix
> +++ a/fs/proc/kcore.c
> @@ -316,7 +316,6 @@ read_kcore(struct file *file, char __use
> 	size_t page_offline_frozen =3D 1;
> 	size_t phdrs_len, notes_len;
> 	struct kcore_list *m;
> -	struct kcore_list *iter;
> 	size_t tsz;
> 	int nphdr;
> 	unsigned long start;
> @@ -480,6 +479,8 @@ read_kcore(struct file *file, char __use
> 		 * the previous entry, search for a matching entry.
> 		 */
> 		if (!m || start < m->addr || start >=3D m->addr + =
m->size) {
> +			struct kcore_list *iter;
> +
> 			m =3D NULL;
> 			list_for_each_entry(iter, &kclist_head, list) {
> 				if (start >=3D iter->addr &&
> _
>=20

	Jakob=