Re: s390/tape: iterator after loop end in tape_assign_minor?

[Heiko Carstens <hca@linux.ibm.com>]

On Tue, May 19, 2026 at 06:00:26PM +0800, Maoyi Xie wrote:
> Hi all,
> 
> While reading drivers/s390/char/tape_core.c I noticed
> something that looks like a past the end iterator pattern.
> I would appreciate it if you could take a look and let me
> know whether this is a real issue, and whether it is worth
> fixing.

Thanks for reporting. This something that Jan should look into.
Full quote below.
 
> The site is tape_assign_minor() (linux-7.1-rc1, around
> line 339):
> 
>     list_for_each_entry(tmp, &tape_device_list, node) {
>             if (minor < tmp->first_minor)
>                     break;
>             minor += TAPE_MINORS_PER_DEV;
>     }
>     if (minor >= 256) {
>             write_unlock(&tape_device_lock);
>             return -ENODEV;
>     }
>     device->first_minor = minor;
>     list_add_tail(&device->node, &tmp->node);
> 
> When the loop walks all entries without break (every
> existing entry has first_minor at or below the candidate
> minor), tmp is past the end. tmp->node then aliases
> tape_device_list (the list head) via container_of offset
> cancellation. list_add_tail(&device->node, &tape_device_list)
> inserts the new device at the tail of the list. That is the
> intended behaviour for a sorted insertion where the new
> device has the largest first_minor. The dereference of the
> past the end iterator is undefined per C11.
> 
> Jakob Koschel cleaned up many such sites in 2022, for
> example commits 99d8ae4ec8a (tracing: Remove usage of list
> iterator variable after the loop), 2966a9918df (clockevents:
> Use dedicated list iterator variable) and dc1acd5c946 (dlm:
> replace usage of found with dedicated list iterator
> variable). This site was not covered.
> 
> A candidate fix would track an explicit insertion target.
> Declare `struct list_head *insert_before = &tape_device_list`
> before the loop. Overwrite it to `&tmp->node` only when the
> loop breaks early. The final list_add_tail then reads
> `insert_before`. On break that points to the entry right
> after the insertion position. On fall-through it stays at
> the list head, so list_add_tail appends at the tail. The
> behaviour is unchanged in all cases, including an empty
> list and a list with one entry.
> 
> If this is intentional or already known, please disregard.
> Otherwise, I am happy to send a [PATCH] or to leave the fix
> to you. Thank you for your time, and sorry for the noise if
> this is not actually worth fixing or has already been
> spotted.
> 
> Thanks,
> Maoyi Xie
> https://maoyixie.com/
>