On Mon, 28 Mar 2022 at 03:09, Xiaomeng Tong <xiam0nd.tong@xxxxxxxxx> wrote: > > on Sun, 27 Mar 2022 16:59:28 +0100, Emil Velikov wrote: > > On Sun, 27 Mar 2022 at 08:39, Xiaomeng Tong <xiam0nd.tong@xxxxxxxxx> wrote: > > > > > > The bug is here: > > > return encoder; > > > > > > The list iterator value 'encoder' will *always* be set and non-NULL > > > by drm_for_each_encoder_mask(), so it is incorrect to assume that the > > > iterator value will be NULL if the list is empty or no element found. > > > Otherwise it will bypass some NULL checks and lead to invalid memory > > > access passing the check. > > > > > > To fix this bug, just return 'encoder' when found, otherwise return > > > NULL. > > > > > > > Isn't this covered by the upcoming list* iterator rework [1] or is > > this another iterator glitch? > > Actually, it is a part of the upcoming work. > > > IMHO we should be looking at fixing the implementation and not the > > hundreds of users through the kernel. > > > > HTH > > -Emil > > [1] https://lwn.net/Articles/887097/ > > Yes, you are right. This has also been taken into account by the upcoming > list iterator rework to avoid a lot uesr' changes as much as possible. > > However, this patch is fixing a potential bug caused by incorrect use of > list iterator outside the loop, which can not be fixed by the implementation > itself. > I see, thanks for the information o/ -Emil