Re: [Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
> On 3. Mar 2022, at 05:58, David Laight wrote:
>
> From: Xiaomeng Tong
>> Sent: 03 March 2022 02:27
>>
>> On Wed, 2 Mar 2022 14:04:06 +, David Laight
>> wrote:
>>> I think that it would be better to make any alternate loop macro
>>> just set the variable to NULL on the loop exit.
>>> That is easier to code for and the compiler might be persuaded to
>>> not redo the test.
>>
>> No, that would lead to a NULL dereference.
>
> Why, it would make it b ethe same as the 'easy to use':
> for (item = head; item; item = item->next) {
> ...
> if (...)
> break;
> ...
> }
> if (!item)
> return;
>
>> The problem is the mis-use of iterator outside the loop on exit, and
>> the iterator will be the HEAD's container_of pointer which pointers
>> to a type-confused struct. Sidenote: The *mis-use* here refers to
>> mistakely access to other members of the struct, instead of the
>> list_head member which acutally is the valid HEAD.
>
> The problem is that the HEAD's container_of pointer should never
> be calculated at all.
> This is what is fundamentally broken about the current definition.
>
>> IOW, you would dereference a (NULL + offset_of_member) address here.
>
> Where?
>
>> Please remind me if i missed something, thanks.
>>
>> Can you share your "alternative definitions" details? thanks!
>
> The loop should probably use as extra variable that points
> to the 'list node' in the next structure.
> Something like:
> for (xxx *iter = head->next;
> iter == ? ((item = NULL),0) : ((item =
> list_item(iter),1));
> iter = item->member->next) {
> ...
> With a bit of casting you can use 'item' to hold 'iter'.
I think this would make sense, it would mean you only assign the containing
element on valid elements.
I was thinking something along the lines of:
#define list_for_each_entry(pos, head, member)
\
for (struct list_head *list = head->next, typeof(pos) pos; \
list == head ? 0 : (( pos = list_entry(pos, list, member), 1));
\
list = list->next)
Although the initialization block of the for loop is not valid C, I'm
not sure there is any way to declare two variables of a different type
in the initialization part of the loop.
I believe all this does is get rid of the >member == (head) check
to terminate the list.
It alone will not fix any of the other issues that using the iterator
variable after the loop currently has.
AFAIK Adrián Moreno is working on doing something along those lines
for the list iterator in openvswitch (that was similar to the kernel
one before) [1].
I *think* they don't declare 'pos' within the loop which we *do want*
to avoid any uses of it after the loop.
(If pos is not declared in the initialization block, shadowing the
*outer* pos, it would just point to the last element of the list or stay
uninitialized if the list is empty).
[1] https://www.mail-archive.com/ovs-dev@openvswitch.org/msg63497.html
>
>>
>>> OTOH there may be alternative definitions that can be used to get
>>> the compiler (or other compiler-like tools) to detect broken code.
>>> Even if the definition can't possibly generate a working kerrnel.
>>
>> The "list_for_each_entry_inside(pos, type, head, member)" way makes
>> the iterator invisiable outside the loop, and would be catched by
>> compiler if use-after-loop things happened.
>
> It is also a compete PITA for anything doing a search.
>
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
>
- Jakob
Re: [Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
> On 1. Mar 2022, at 18:36, Greg KH wrote: > > On Tue, Mar 01, 2022 at 12:28:15PM +0100, Jakob Koschel wrote: >> >> >>> On 1. Mar 2022, at 01:41, Linus Torvalds >>> wrote: >>> >>> On Mon, Feb 28, 2022 at 1:47 PM Jakob Koschel >>> wrote: >>>> >>>> The goal of this is to get compiler warnings right? This would indeed be >>>> great. >>> >>> Yes, so I don't mind having a one-time patch that has been gathered >>> using some automated checker tool, but I don't think that works from a >>> long-term maintenance perspective. >>> >>> So if we have the basic rule being "don't use the loop iterator after >>> the loop has finished, because it can cause all kinds of subtle >>> issues", then in _addition_ to fixing the existing code paths that >>> have this issue, I really would want to (a) get a compiler warning for >>> future cases and (b) make it not actually _work_ for future cases. >>> >>> Because otherwise it will just happen again. >>> >>>> Changing the list_for_each_entry() macro first will break all of those >>>> cases >>>> (e.g. the ones using 'list_entry_is_head()). >>> >>> So I have no problems with breaking cases that we basically already >>> have a patch for due to your automated tool. There were certainly >>> more than a handful, but it didn't look _too_ bad to just make the >>> rule be "don't use the iterator after the loop". >>> >>> Of course, that's just based on that patch of yours. Maybe there are a >>> ton of other cases that your patch didn't change, because they didn't >>> match your trigger case, so I may just be overly optimistic here. >> >> Based on the coccinelle script there are ~480 cases that need fixing >> in total. I'll now finish all of them and then split them by >> submodules as Greg suggested and repost a patch set per submodule. >> Sounds good? > > Sounds good to me! > > If you need help carving these up and maintaining them over time as > different subsystem maintainers accept/ignore them, just let me know. > Doing large patchsets like this can be tough without a lot of > experience. Very much appreciated! There will probably be some cases that do not match one of the pattern we already discussed and need separate attention. I was planning to start with one subsystem and adjust the coming ones according to the feedback gather there instead of posting all of them in one go. > > thanks, > > greg k-h - Jakob
Re: [Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
> On 1. Mar 2022, at 01:41, Linus Torvalds > wrote: > > On Mon, Feb 28, 2022 at 1:47 PM Jakob Koschel wrote: >> >> The goal of this is to get compiler warnings right? This would indeed be >> great. > > Yes, so I don't mind having a one-time patch that has been gathered > using some automated checker tool, but I don't think that works from a > long-term maintenance perspective. > > So if we have the basic rule being "don't use the loop iterator after > the loop has finished, because it can cause all kinds of subtle > issues", then in _addition_ to fixing the existing code paths that > have this issue, I really would want to (a) get a compiler warning for > future cases and (b) make it not actually _work_ for future cases. > > Because otherwise it will just happen again. > >> Changing the list_for_each_entry() macro first will break all of those cases >> (e.g. the ones using 'list_entry_is_head()). > > So I have no problems with breaking cases that we basically already > have a patch for due to your automated tool. There were certainly > more than a handful, but it didn't look _too_ bad to just make the > rule be "don't use the iterator after the loop". > > Of course, that's just based on that patch of yours. Maybe there are a > ton of other cases that your patch didn't change, because they didn't > match your trigger case, so I may just be overly optimistic here. Based on the coccinelle script there are ~480 cases that need fixing in total. I'll now finish all of them and then split them by submodules as Greg suggested and repost a patch set per submodule. Sounds good? > > But basically to _me_, the important part is that the end result is > maintainable longer-term. I'm more than happy to have a one-time patch > to fix a lot of dubious cases if we can then have clean rules going > forward. > >> I assumed it is better to fix those cases first and then have a simple >> coccinelle script changing the macro + moving the iterator into the scope >> of the macro. > > So that had been another plan of mine, until I actually looked at > changing the macro. In the one case I looked at, it was ugly beyond > belief. > > It turns out that just syntactically, it's really nice to give the > type of the iterator from outside the way we do now. Yeah, it may be a > bit odd, and maybe it's partly because I'm so used to the > "list_for_each_list_entry()" syntax, but moving the type into the loop > construct really made it nasty - either one very complex line, or > having to split it over two lines which was even worse. > > Maybe the place I looked at just happened to have a long typename, but > it's basically always going to be a struct, so it's never a _simple_ > type. And it just looked very odd adn unnatural to have the type as > one of the "arguments" to that list_for_each_entry() macro. > > So yes, initially my idea had been to just move the iterator entirely > inside the macro. But specifying the type got so ugly that I think > that > >typeof (pos) pos > > trick inside the macro really ends up giving us the best of all worlds: > > (a) let's us keep the existing syntax and code for all the nice cases > that did everything inside the loop anyway > > (b) gives us a nice warning for any normal use-after-loop case > (unless you explicitly initialized it like that > sgx_mmu_notifier_release() function did for no good reason > > (c) also guarantees that even if you don't get a warning, > non-converted (or newly written) bad code won't actually _work_ > > so you end up getting the new rules without any ambiguity or mistaken > >> With this you are no longer able to set the 'outer' pos within the list >> iterator loop body or am I missing something? > > Correct. Any assignment inside the loop will be entirely just to the > local loop case. So any "break;" out of the loop will have to set > another variable - like your updated patch did. > >> I fail to see how this will make most of the changes in this >> patch obsolete (if that was the intention). > > I hope my explanation above clarifies my thinking: I do not dislike > your patch, and in fact your patch is indeed required to make the new > semantics work. ok it's all clear now, thanks for clarifying. I've defined all the 'tmp' iterator variables uninitialized so applying your patch on top of that later will just give the nice compiler warning if they are used past the loop body. > > What I disliked was always the maintainability of your patch - making > the rules be something that isn't actually visible in the source code, > and letting the old semantics still work
Re: [Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
> On 28. Feb 2022, at 21:10, Linus Torvalds > wrote: > > On Mon, Feb 28, 2022 at 12:03 PM Linus Torvalds > wrote: >> >> Side note: we do need *some* way to do it. > > Ooh. > > This patch is a work of art. > > And I mean that in the worst possible way. > > We can do > >typeof(pos) pos > > in the 'for ()' loop, and never use __iter at all. > > That means that inside the for-loop, we use a _different_ 'pos' than outside. > > And then the compiler will not see some "might be uninitialized", but > the outer 'pos' *will* be uninitialized. > > Unless, of course, the outer 'pos' had that pointless explicit initializer. The goal of this is to get compiler warnings right? This would indeed be great. Changing the list_for_each_entry() macro first will break all of those cases (e.g. the ones using 'list_entry_is_head()). I assumed it is better to fix those cases first and then have a simple coccinelle script changing the macro + moving the iterator into the scope of the macro. > > Here - can somebody poke holes in this "work of art" patch? With this you are no longer able to set the 'outer' pos within the list iterator loop body or am I missing something? Like this it stays uninitialized but you'll probably want to set it from within the loop. You would then yet again need a variable with another name to use after the loop. I fail to see how this will make most of the changes in this patch obsolete (if that was the intention). > > Linus > - Jakob
Re: [Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
> On 28. Feb 2022, at 21:56, Christian König wrote:
>
>
>
> Am 28.02.22 um 21:42 schrieb James Bottomley:
>> On Mon, 2022-02-28 at 21:07 +0100, Christian König wrote:
>>> Am 28.02.22 um 20:56 schrieb Linus Torvalds:
On Mon, Feb 28, 2022 at 4:19 AM Christian König
wrote:
[SNIP]
Anybody have any ideas?
>>> I think we should look at the use cases why code is touching (pos)
>>> after the loop.
>>>
>>> Just from skimming over the patches to change this and experience
>>> with the drivers/subsystems I help to maintain I think the primary
>>> pattern looks something like this:
>>>
>>> list_for_each_entry(entry, head, member) {
>>> if (some_condition_checking(entry))
>>> break;
>>> }
>>> do_something_with(entry);
There are other cases where the list iterator variable is used after the loop
Some examples:
- list_for_each_entry_continue() and list_for_each_entry_from().
- (although very rare) the head is actually of the correct struct type.
(ppc440spe_get_group_entry(): drivers/dma/ppc4xx/adma.c:1436)
- to use pos->list for example for list_add_tail():
(add_static_vm_early(): arch/arm/mm/ioremap.c:107)
If the scope of the list iterator is limited those still need fixing in a
different way.
>>
>> Actually, we usually have a check to see if the loop found anything,
>> but in that case it should something like
>>
>> if (list_entry_is_head(entry, head, member)) {
>> return with error;
>> }
>> do_somethin_with(entry);
>>
>> Suffice? The list_entry_is_head() macro is designed to cope with the
>> bogus entry on head problem.
>
> That will work and is also what people already do.
>
> The key problem is that we let people do the same thing over and over again
> with slightly different implementations.
>
> Out in the wild I've seen at least using a separate variable, using a bool to
> indicate that something was found and just assuming that the list has an
> entry.
>
> The last case is bogus and basically what can break badly.
>
> If we would have an unified macro which search for an entry combined with
> automated reporting on patches to use that macro I think the potential to
> introduce such issues will already go down massively without auditing tons of
> existing code.
Having a unified way to do the same thing would indeed be great.
>
> Regards,
> Christian.
>
>>
>> James
>>
>>
>
- Jakob
[Intel-gfx] [PATCH 0/6] Remove usage of list iterator past the loop body
This is the first patch removing several categories of use cases of
the list iterator variable past the loop.
This is follow up to the discussion in:
https://lore.kernel.org/all/20220217184829.1991035-1-jakobkosc...@gmail.com/
As concluded in:
https://lore.kernel.org/all/yhdfeiwi4edth...@kroah.com/
the correct use should be using a separate variable after the loop
and using a 'tmp' variable as the list iterator.
The list iterator will not point to a valid structure after the loop
if no break/goto was hit. Invalid uses of the list iterator variable
can be avoided altogether by simply using a separate pointer to
iterate the list.
Linus and Greg agreed on the following pattern:
- struct gr_request *req;
+ struct gr_request *req = NULL;
+ struct gr_request *tmp;
struct gr_ep *ep;
int ret = 0;
- list_for_each_entry(req, >queue, queue) {
- if (>req == _req)
+ list_for_each_entry(tmp, >queue, queue) {
+ if (>req == _req) {
+ req = tmp;
break;
+ }
}
- if (>req != _req) {
+ if (!req) {
ret = -EINVAL;
goto out;
}
With gnu89 the list iterator variable cannot yet be declared
within the for loop of the list iterator.
Moving to a more modern version of C would allow defining
the list iterator variable within the macro, limiting
the scope to the loop.
This avoids any incorrect usage past the loop altogether.
This are around 30% of the cases where the iterator
variable is used past the loop (identified with a slightly
modified version of use_after_iter.cocci).
I've decided to split it into at a few patches separated
by similar use cases.
Because the output of get_maintainer.pl was too big,
I included all the found lists and everyone from the
previous discussion.
Jakob Koschel (6):
drivers: usb: remove usage of list iterator past the loop body
treewide: remove using list iterator after loop body as a ptr
treewide: fix incorrect use to determine if list is empty
drivers: remove unnecessary use of list iterator variable
treewide: remove dereference of list iterator after loop body
treewide: remove check of list iterator against head past the loop
body
arch/arm/mach-mmp/sram.c | 9 ++--
arch/arm/plat-pxa/ssp.c | 28 +---
arch/powerpc/sysdev/fsl_gtm.c | 4 +-
arch/x86/kernel/cpu/sgx/encl.c| 6 ++-
drivers/block/drbd/drbd_req.c | 45 ---
drivers/counter/counter-chrdev.c | 26 ++-
drivers/crypto/cavium/nitrox/nitrox_main.c| 11 +++--
drivers/dma/dw-edma/dw-edma-core.c| 4 +-
drivers/dma/ppc4xx/adma.c | 11 +++--
drivers/firewire/core-transaction.c | 32 +++--
drivers/firewire/sbp2.c | 14 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 19 +---
drivers/gpu/drm/drm_memory.c | 15 ---
drivers/gpu/drm/drm_mm.c | 17 ---
drivers/gpu/drm/drm_vm.c | 13 +++---
drivers/gpu/drm/gma500/oaktrail_lvds.c| 9 ++--
drivers/gpu/drm/i915/gem/i915_gem_context.c | 14 +++---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 15 ---
drivers/gpu/drm/i915/gt/intel_ring.c | 15 ---
.../gpu/drm/nouveau/nvkm/subdev/clk/base.c| 11 +++--
.../gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c | 13 +++---
drivers/gpu/drm/scheduler/sched_main.c| 14 +++---
drivers/gpu/drm/ttm/ttm_bo.c | 19
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +
drivers/infiniband/hw/hfi1/tid_rdma.c | 16 ---
drivers/infiniband/hw/mlx4/main.c | 12 ++---
drivers/media/dvb-frontends/mxl5xx.c | 11 +++--
drivers/media/pci/saa7134/saa7134-alsa.c | 4 +-
drivers/media/v4l2-core/v4l2-ctrls-api.c | 31 +++--
drivers/misc/mei/interrupt.c | 12 ++---
.../net/ethernet/intel/i40e/i40e_ethtool.c| 3 +-
.../net/ethernet/qlogic/qede/qede_filter.c| 11 +++--
drivers/net/wireless/ath/ath6kl/htc_mbox.c| 2 +-
.../net/wireless/intel/ipw2x00/libipw_rx.c| 15 ---
drivers/perf/xgene_pmu.c | 13 +++---
drivers/power/supply/cpcap-battery.c | 11 +++--
drivers/scsi/lpfc/lpfc_bsg.c | 16 ---
drivers/scsi/scsi_transport_sas.c | 17 ---
drivers/scsi/wd719x.c | 12 +++--
drivers/staging/rtl8192e/rtl819x_TSProc.c | 17 +++
drivers/staging/rtl8192e/rtllib_rx.c | 17 ---
.../staging/rtl8192u/ieee80211/ieee80211_rx.c | 15 ---
.../rtl8192u/ieee80211/rtl819x_TSProc.c | 19
drivers/thermal/thermal_core.c| 38 ++--
drivers/usb/gadget/composite.c| 9 ++--
dri
Re: [Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
> On 28. Feb 2022, at 12:20, Greg KH wrote: > > On Mon, Feb 28, 2022 at 12:08:18PM +0100, Jakob Koschel wrote: >> If the list does not contain the expected element, the value of >> list_for_each_entry() iterator will not point to a valid structure. >> To avoid type confusion in such case, the list iterator >> scope will be limited to list_for_each_entry() loop. >> >> In preparation to limiting scope of a list iterator to the list traversal >> loop, use a dedicated pointer to point to the found element. >> Determining if an element was found is then simply checking if >> the pointer is != NULL. >> >> Signed-off-by: Jakob Koschel >> --- >> arch/x86/kernel/cpu/sgx/encl.c | 6 +++-- >> drivers/scsi/scsi_transport_sas.c| 17 - >> drivers/thermal/thermal_core.c | 38 ++-- >> drivers/usb/gadget/configfs.c| 22 ++-- >> drivers/usb/gadget/udc/max3420_udc.c | 11 +--- >> drivers/usb/gadget/udc/tegra-xudc.c | 11 +--- >> drivers/usb/mtu3/mtu3_gadget.c | 11 +--- >> drivers/usb/musb/musb_gadget.c | 11 +--- >> drivers/vfio/mdev/mdev_core.c| 11 +--- >> 9 files changed, 88 insertions(+), 50 deletions(-) > > The drivers/usb/ portion of this patch should be in patch 1/X, right? I kept them separate since it's a slightly different case. Using 'ptr' instead of '>other_member'. Regardless, I'll split this commit up as you mentioned. > > Also, you will have to split these up per-subsystem so that the > different subsystem maintainers can take these in their trees. Thanks for the feedback. To clarify I understand you correctly: I should do one patch set per-subsystem with every instance/(file?) fixed as a separate commit? If I understand correctly, I'll repost accordingly. > > thanks, > > greg k-h thanks, Jakob Koschel
Re: [Intel-gfx] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body
> On 28. Feb 2022, at 12:24, Dan Carpenter wrote:
>
> On Mon, Feb 28, 2022 at 12:08:17PM +0100, Jakob Koschel wrote:
>> diff --git a/drivers/usb/gadget/udc/at91_udc.c
>> b/drivers/usb/gadget/udc/at91_udc.c
>> index 9040a0561466..0fd0307bc07b 100644
>> --- a/drivers/usb/gadget/udc/at91_udc.c
>> +++ b/drivers/usb/gadget/udc/at91_udc.c
>> @@ -150,13 +150,14 @@ static void proc_ep_show(struct seq_file *s, struct
>> at91_ep *ep)
>> if (list_empty (>queue))
>> seq_printf(s, "\t(queue empty)\n");
>>
>> -else list_for_each_entry (req, >queue, queue) {
>> -unsignedlength = req->req.actual;
>> +else
>> +list_for_each_entry(req, >queue, queue) {
>> +unsigned intlength = req->req.actual;
>>
>> -seq_printf(s, "\treq %p len %d/%d buf %p\n",
>> ->req, length,
>> -req->req.length, req->req.buf);
>> -}
>> +seq_printf(s, "\treq %p len %d/%d buf %p\n",
>> +>req, length,
>> +req->req.length, req->req.buf);
>> +}
>
> Don't make unrelated white space changes. It just makes the patch
> harder to review. As you're writing the patch make note of any
> additional changes and do them later in a separate patch.
>
> Also a multi-line indent gets curly braces for readability even though
> it's not required by C. And then both sides would get curly braces.
>
>> spin_unlock_irqrestore(>lock, flags);
>> }
>>
>> @@ -226,7 +227,7 @@ static int proc_udc_show(struct seq_file *s, void
>> *unused)
>>
>> if (udc->enabled && udc->vbus) {
>> proc_ep_show(s, >ep[0]);
>> -list_for_each_entry (ep, >gadget.ep_list, ep.ep_list) {
>> +list_for_each_entry(ep, >gadget.ep_list, ep.ep_list) {
>
> Another unrelated change.
>
>> if (ep->ep.desc)
>> proc_ep_show(s, ep);
>> }
>
>
> [ snip ]
Thanks for pointing out, I'll remove the changes here and note them down
to send them separately.
>
>> diff --git a/drivers/usb/gadget/udc/net2272.c
>> b/drivers/usb/gadget/udc/net2272.c
>> index 7c38057dcb4a..bb59200f1596 100644
>> --- a/drivers/usb/gadget/udc/net2272.c
>> +++ b/drivers/usb/gadget/udc/net2272.c
>> @@ -926,7 +926,8 @@ static int
>> net2272_dequeue(struct usb_ep *_ep, struct usb_request *_req)
>> {
>> struct net2272_ep *ep;
>> -struct net2272_request *req;
>> +struct net2272_request *req = NULL;
>> +struct net2272_request *tmp;
>> unsigned long flags;
>> int stopped;
>>
>> @@ -939,11 +940,13 @@ net2272_dequeue(struct usb_ep *_ep, struct usb_request
>> *_req)
>> ep->stopped = 1;
>>
>> /* make sure it's still queued on this endpoint */
>> -list_for_each_entry(req, >queue, queue) {
>> -if (>req == _req)
>> +list_for_each_entry(tmp, >queue, queue) {
>> +if (>req == _req) {
>> +req = tmp;
>> break;
>> +}
>> }
>> -if (>req != _req) {
>> +if (!req) {
>> ep->stopped = stopped;
>> spin_unlock_irqrestore(>dev->lock, flags);
>> return -EINVAL;
>> @@ -954,7 +957,6 @@ net2272_dequeue(struct usb_ep *_ep, struct usb_request
>> *_req)
>> dev_dbg(ep->dev->dev, "unlink (%s) pio\n", _ep->name);
>> net2272_done(ep, req, -ECONNRESET);
>> }
>> -req = NULL;
>
> Another unrelated change. These are all good changes but send them as
> separate patches.
You are referring to the req = NULL, right?
I've changed the use of 'req' in the same function and assumed that I can
just remove the unnecessary statement. But if it's better to do separately
I'll do that.
>
>> ep->stopped = stopped;
>>
>> spin_unlock_irqrestore(>dev->lock, flags);
>
> regards,
> dan carpenter
thanks,
Jakob Koschel
[Intel-gfx] [PATCH 5/6] treewide: remove dereference of list iterator after loop body
The list iterator variable will be a bogus pointer if no break was hit.
Dereferencing it could load *any* out-of-bounds/undefined value
making it unsafe to use that in the comparision to determine if the
specific element was found.
This is fixed by using a separate list iterator variable for the loop
and only setting the original variable if a suitable element was found.
Then determing if the element was found is simply checking if the
variable is set.
Signed-off-by: Jakob Koschel
---
drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c | 11 +++
drivers/scsi/wd719x.c | 12
fs/f2fs/segment.c | 9 ++---
3 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
index 57199be082fd..c56cd9e59a66 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/clk/base.c
@@ -471,20 +471,23 @@ nvkm_pstate_new(struct nvkm_clk *clk, int idx)
static int
nvkm_clk_ustate_update(struct nvkm_clk *clk, int req)
{
- struct nvkm_pstate *pstate;
+ struct nvkm_pstate *pstate = NULL;
+ struct nvkm_pstate *tmp;
int i = 0;
if (!clk->allow_reclock)
return -ENOSYS;
if (req != -1 && req != -2) {
- list_for_each_entry(pstate, >states, head) {
- if (pstate->pstate == req)
+ list_for_each_entry(tmp, >states, head) {
+ if (tmp->pstate == req) {
+ pstate = tmp;
break;
+ }
i++;
}
- if (pstate->pstate != req)
+ if (!pstate)
return -EINVAL;
req = i;
}
diff --git a/drivers/scsi/wd719x.c b/drivers/scsi/wd719x.c
index 1a7947554581..be270ed8e00d 100644
--- a/drivers/scsi/wd719x.c
+++ b/drivers/scsi/wd719x.c
@@ -684,11 +684,15 @@ static irqreturn_t wd719x_interrupt(int irq, void *dev_id)
case WD719X_INT_SPIDERFAILED:
/* was the cmd completed a direct or SCB command? */
if (regs.bytes.OPC == WD719X_CMD_PROCESS_SCB) {
- struct wd719x_scb *scb;
- list_for_each_entry(scb, >active_scbs, list)
- if (SCB_out == scb->phys)
+ struct wd719x_scb *scb = NULL;
+ struct wd719x_scb *tmp;
+
+ list_for_each_entry(tmp, >active_scbs, list)
+ if (SCB_out == tmp->phys) {
+ scb = tmp;
break;
- if (SCB_out == scb->phys)
+ }
+ if (scb)
wd719x_interrupt_SCB(wd, regs, scb);
else
dev_err(>pdev->dev, "card returned invalid
SCB pointer\n");
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 1dabc8244083..a3684385e04a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -356,16 +356,19 @@ void f2fs_drop_inmem_page(struct inode *inode, struct
page *page)
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
struct list_head *head = >inmem_pages;
struct inmem_pages *cur = NULL;
+ struct inmem_pages *tmp;
f2fs_bug_on(sbi, !page_private_atomic(page));
mutex_lock(>inmem_lock);
- list_for_each_entry(cur, head, list) {
- if (cur->page == page)
+ list_for_each_entry(tmp, head, list) {
+ if (tmp->page == page) {
+ cur = tmp;
break;
+ }
}
- f2fs_bug_on(sbi, list_empty(head) || cur->page != page);
+ f2fs_bug_on(sbi, !cur);
list_del(>list);
mutex_unlock(>inmem_lock);
--
2.25.1
[Intel-gfx] [PATCH 2/6] treewide: remove using list iterator after loop body as a ptr
If the list does not contain the expected element, the value of
list_for_each_entry() iterator will not point to a valid structure.
To avoid type confusion in such case, the list iterator
scope will be limited to list_for_each_entry() loop.
In preparation to limiting scope of a list iterator to the list traversal
loop, use a dedicated pointer to point to the found element.
Determining if an element was found is then simply checking if
the pointer is != NULL.
Signed-off-by: Jakob Koschel
---
arch/x86/kernel/cpu/sgx/encl.c | 6 +++--
drivers/scsi/scsi_transport_sas.c| 17 -
drivers/thermal/thermal_core.c | 38 ++--
drivers/usb/gadget/configfs.c| 22 ++--
drivers/usb/gadget/udc/max3420_udc.c | 11 +---
drivers/usb/gadget/udc/tegra-xudc.c | 11 +---
drivers/usb/mtu3/mtu3_gadget.c | 11 +---
drivers/usb/musb/musb_gadget.c | 11 +---
drivers/vfio/mdev/mdev_core.c| 11 +---
9 files changed, 88 insertions(+), 50 deletions(-)
diff --git a/arch/x86/kernel/cpu/sgx/encl.c b/arch/x86/kernel/cpu/sgx/encl.c
index 48afe96ae0f0..6c916416decc 100644
--- a/arch/x86/kernel/cpu/sgx/encl.c
+++ b/arch/x86/kernel/cpu/sgx/encl.c
@@ -450,7 +450,8 @@ static void sgx_mmu_notifier_release(struct mmu_notifier
*mn,
struct mm_struct *mm)
{
struct sgx_encl_mm *encl_mm = container_of(mn, struct sgx_encl_mm,
mmu_notifier);
- struct sgx_encl_mm *tmp = NULL;
+ struct sgx_encl_mm *found_encl_mm = NULL;
+ struct sgx_encl_mm *tmp;
/*
* The enclave itself can remove encl_mm. Note, objects can't be moved
@@ -460,12 +461,13 @@ static void sgx_mmu_notifier_release(struct mmu_notifier
*mn,
list_for_each_entry(tmp, _mm->encl->mm_list, list) {
if (tmp == encl_mm) {
list_del_rcu(_mm->list);
+ found_encl_mm = tmp;
break;
}
}
spin_unlock(_mm->encl->mm_lock);
- if (tmp == encl_mm) {
+ if (found_encl_mm) {
synchronize_srcu(_mm->encl->srcu);
mmu_notifier_put(mn);
}
diff --git a/drivers/scsi/scsi_transport_sas.c
b/drivers/scsi/scsi_transport_sas.c
index 4ee578b181da..a8cbd90db9d2 100644
--- a/drivers/scsi/scsi_transport_sas.c
+++ b/drivers/scsi/scsi_transport_sas.c
@@ -1060,26 +1060,29 @@ EXPORT_SYMBOL(sas_port_get_phy);
* connected to a remote device is a port, so ports must be formed on
* all devices with phys if they're connected to anything.
*/
-void sas_port_add_phy(struct sas_port *port, struct sas_phy *phy)
+void sas_port_add_phy(struct sas_port *port, struct sas_phy *_phy)
{
mutex_lock(>phy_list_mutex);
- if (unlikely(!list_empty(>port_siblings))) {
+ if (unlikely(!list_empty(&_phy->port_siblings))) {
/* make sure we're already on this port */
+ struct sas_phy *phy = NULL;
struct sas_phy *tmp;
list_for_each_entry(tmp, >phy_list, port_siblings)
- if (tmp == phy)
+ if (tmp == _phy) {
+ phy = tmp;
break;
+ }
/* If this trips, you added a phy that was already
* part of a different port */
- if (unlikely(tmp != phy)) {
+ if (unlikely(!phy)) {
dev_printk(KERN_ERR, >dev, "trying to add phy %s
fails: it's already part of another port\n",
- dev_name(>dev));
+ dev_name(&_phy->dev));
BUG();
}
} else {
- sas_port_create_link(port, phy);
- list_add_tail(>port_siblings, >phy_list);
+ sas_port_create_link(port, _phy);
+ list_add_tail(&_phy->port_siblings, >phy_list);
port->num_phys++;
}
mutex_unlock(>phy_list_mutex);
diff --git a/drivers/thermal/thermal_core.c b/drivers/thermal/thermal_core.c
index 82654dc8382b..97198543448b 100644
--- a/drivers/thermal/thermal_core.c
+++ b/drivers/thermal/thermal_core.c
@@ -625,24 +625,30 @@ int thermal_zone_bind_cooling_device(struct
thermal_zone_device *tz,
{
struct thermal_instance *dev;
struct thermal_instance *pos;
- struct thermal_zone_device *pos1;
- struct thermal_cooling_device *pos2;
+ struct thermal_zone_device *pos1 = NULL;
+ struct thermal_zone_device *tmp1;
+ struct thermal_cooling_device *pos2 = NULL;
+ struct thermal_cooling_device *tmp2;
unsigned long max_state;
int result, ret;
if (trip >= tz->trips || trip < 0)
return -EINVAL;
- list_for_each_ent
[Intel-gfx] [PATCH 3/6] treewide: fix incorrect use to determine if list is empty
The list iterator value will *always* be set by list_for_each_entry().
It is incorrect to assume that the iterator value will be NULL if the
list is empty.
Instead of checking the pointer it should be checked if
the list is empty.
In acpi_get_pmu_hw_inf() instead of setting the pointer to NULL
on the break, it is set to the correct value and leaving it
NULL if no element was found.
Signed-off-by: Jakob Koschel
---
arch/powerpc/sysdev/fsl_gtm.c| 4 ++--
drivers/media/pci/saa7134/saa7134-alsa.c | 4 ++--
drivers/perf/xgene_pmu.c | 13 +++--
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/arch/powerpc/sysdev/fsl_gtm.c b/arch/powerpc/sysdev/fsl_gtm.c
index 8963eaffb1b7..39186ad6b3c3 100644
--- a/arch/powerpc/sysdev/fsl_gtm.c
+++ b/arch/powerpc/sysdev/fsl_gtm.c
@@ -86,7 +86,7 @@ static LIST_HEAD(gtms);
*/
struct gtm_timer *gtm_get_timer16(void)
{
- struct gtm *gtm = NULL;
+ struct gtm *gtm;
int i;
list_for_each_entry(gtm, , list_node) {
@@ -103,7 +103,7 @@ struct gtm_timer *gtm_get_timer16(void)
spin_unlock_irq(>lock);
}
- if (gtm)
+ if (!list_empty())
return ERR_PTR(-EBUSY);
return ERR_PTR(-ENODEV);
}
diff --git a/drivers/media/pci/saa7134/saa7134-alsa.c
b/drivers/media/pci/saa7134/saa7134-alsa.c
index fb24d2ed3621..d3cde05a6eba 100644
--- a/drivers/media/pci/saa7134/saa7134-alsa.c
+++ b/drivers/media/pci/saa7134/saa7134-alsa.c
@@ -1214,7 +1214,7 @@ static int alsa_device_exit(struct saa7134_dev *dev)
static int saa7134_alsa_init(void)
{
- struct saa7134_dev *dev = NULL;
+ struct saa7134_dev *dev;
saa7134_dmasound_init = alsa_device_init;
saa7134_dmasound_exit = alsa_device_exit;
@@ -1229,7 +1229,7 @@ static int saa7134_alsa_init(void)
alsa_device_init(dev);
}
- if (dev == NULL)
+ if (list_empty(_devlist))
pr_info("saa7134 ALSA: no saa7134 cards found\n");
return 0;
diff --git a/drivers/perf/xgene_pmu.c b/drivers/perf/xgene_pmu.c
index 2b6d476bd213..e255f9e665d1 100644
--- a/drivers/perf/xgene_pmu.c
+++ b/drivers/perf/xgene_pmu.c
@@ -1460,7 +1460,8 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu
*xgene_pmu,
struct hw_pmu_info *inf;
void __iomem *dev_csr;
struct resource res;
- struct resource_entry *rentry;
+ struct resource_entry *rentry = NULL;
+ struct resource_entry *tmp;
int enable_bit;
int rc;
@@ -1475,16 +1476,16 @@ xgene_pmu_dev_ctx *acpi_get_pmu_hw_inf(struct xgene_pmu
*xgene_pmu,
return NULL;
}
- list_for_each_entry(rentry, _list, node) {
- if (resource_type(rentry->res) == IORESOURCE_MEM) {
- res = *rentry->res;
- rentry = NULL;
+ list_for_each_entry(tmp, _list, node) {
+ if (resource_type(tmp->res) == IORESOURCE_MEM) {
+ res = *tmp->res;
+ rentry = tmp;
break;
}
}
acpi_dev_free_resource_list(_list);
- if (rentry) {
+ if (!rentry) {
dev_err(dev, "PMU type %d: No memory resource found\n", type);
return NULL;
}
--
2.25.1
[Intel-gfx] [PATCH 1/6] drivers: usb: remove usage of list iterator past the loop body
If the list representing the request queue does not contain the expected
request, the value of list_for_each_entry() iterator will not point to a
valid structure. To avoid type confusion in such case, the list iterator
scope will be limited to list_for_each_entry() loop.
In preparation to limiting scope of a list iterator to the list traversal
loop, use a dedicated pointer to point to the found request object.
Signed-off-by: Jakob Koschel
---
drivers/usb/gadget/udc/aspeed-vhub/epn.c | 11 ++
drivers/usb/gadget/udc/at91_udc.c| 26 ++--
drivers/usb/gadget/udc/atmel_usba_udc.c | 11 ++
drivers/usb/gadget/udc/bdc/bdc_ep.c | 11 +++---
drivers/usb/gadget/udc/fsl_qe_udc.c | 11 ++
drivers/usb/gadget/udc/fsl_udc_core.c| 11 ++
drivers/usb/gadget/udc/goku_udc.c| 11 ++
drivers/usb/gadget/udc/gr_udc.c | 11 ++
drivers/usb/gadget/udc/lpc32xx_udc.c | 11 ++
drivers/usb/gadget/udc/mv_u3d_core.c | 11 ++
drivers/usb/gadget/udc/mv_udc_core.c | 11 ++
drivers/usb/gadget/udc/net2272.c | 12 ++-
drivers/usb/gadget/udc/net2280.c | 11 ++
drivers/usb/gadget/udc/omap_udc.c| 11 ++
drivers/usb/gadget/udc/pxa25x_udc.c | 11 ++
drivers/usb/gadget/udc/s3c-hsudc.c | 11 ++
drivers/usb/gadget/udc/udc-xilinx.c | 11 ++
17 files changed, 128 insertions(+), 75 deletions(-)
diff --git a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
index 917892ca8753..cad874ee4472 100644
--- a/drivers/usb/gadget/udc/aspeed-vhub/epn.c
+++ b/drivers/usb/gadget/udc/aspeed-vhub/epn.c
@@ -466,19 +466,22 @@ static int ast_vhub_epn_dequeue(struct usb_ep* u_ep,
struct usb_request *u_req)
{
struct ast_vhub_ep *ep = to_ast_ep(u_ep);
struct ast_vhub *vhub = ep->vhub;
- struct ast_vhub_req *req;
+ struct ast_vhub_req *req = NULL;
+ struct ast_vhub_req *tmp;
unsigned long flags;
int rc = -EINVAL;
spin_lock_irqsave(>lock, flags);
/* Make sure it's actually queued on this endpoint */
- list_for_each_entry (req, >queue, queue) {
- if (>req == u_req)
+ list_for_each_entry(tmp, >queue, queue) {
+ if (>req == u_req) {
+ req = tmp;
break;
+ }
}
- if (>req == u_req) {
+ if (req) {
EPVDBG(ep, "dequeue req @%p active=%d\n",
req, req->active);
if (req->active)
diff --git a/drivers/usb/gadget/udc/at91_udc.c
b/drivers/usb/gadget/udc/at91_udc.c
index 9040a0561466..0fd0307bc07b 100644
--- a/drivers/usb/gadget/udc/at91_udc.c
+++ b/drivers/usb/gadget/udc/at91_udc.c
@@ -150,13 +150,14 @@ static void proc_ep_show(struct seq_file *s, struct
at91_ep *ep)
if (list_empty (>queue))
seq_printf(s, "\t(queue empty)\n");
- else list_for_each_entry (req, >queue, queue) {
- unsignedlength = req->req.actual;
+ else
+ list_for_each_entry(req, >queue, queue) {
+ unsigned intlength = req->req.actual;
- seq_printf(s, "\treq %p len %d/%d buf %p\n",
- >req, length,
- req->req.length, req->req.buf);
- }
+ seq_printf(s, "\treq %p len %d/%d buf %p\n",
+ >req, length,
+ req->req.length, req->req.buf);
+ }
spin_unlock_irqrestore(>lock, flags);
}
@@ -226,7 +227,7 @@ static int proc_udc_show(struct seq_file *s, void *unused)
if (udc->enabled && udc->vbus) {
proc_ep_show(s, >ep[0]);
- list_for_each_entry (ep, >gadget.ep_list, ep.ep_list) {
+ list_for_each_entry(ep, >gadget.ep_list, ep.ep_list) {
if (ep->ep.desc)
proc_ep_show(s, ep);
}
@@ -704,7 +705,8 @@ static int at91_ep_queue(struct usb_ep *_ep,
static int at91_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
{
struct at91_ep *ep;
- struct at91_request *req;
+ struct at91_request *req = NULL;
+ struct at91_request *tmp;
unsigned long flags;
struct at91_udc *udc;
@@ -717,11 +719,13 @@ static int at91_ep_dequeue(struct usb_ep *_ep, struct
usb_request *_req)
spin_lock_irqsave(>lock, flags);
/* make sure it's actually queued on this endpoint */
- list_for_each_entry (req, >queue, queue) {
- if (>req == _req)
+ list
[Intel-gfx] [PATCH 6/6] treewide: remove check of list iterator against head past the loop body
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.
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
>member == head, using the iterator variable after the loop should
be avoided.
In preparation to limiting the scope of a list iterator to the list
traversal loop, use a dedicated pointer to point to the found element.
Signed-off-by: Jakob Koschel
---
arch/arm/mach-mmp/sram.c | 9 ++--
arch/arm/plat-pxa/ssp.c | 28 +---
drivers/block/drbd/drbd_req.c | 45 ---
drivers/counter/counter-chrdev.c | 26 ++-
drivers/crypto/cavium/nitrox/nitrox_main.c| 11 +++--
drivers/dma/ppc4xx/adma.c | 11 +++--
drivers/firewire/core-transaction.c | 32 +++--
drivers/firewire/sbp2.c | 14 +++---
drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c| 19 +---
drivers/gpu/drm/drm_memory.c | 15 ---
drivers/gpu/drm/drm_mm.c | 17 ---
drivers/gpu/drm/drm_vm.c | 13 +++---
drivers/gpu/drm/gma500/oaktrail_lvds.c| 9 ++--
drivers/gpu/drm/i915/gem/i915_gem_context.c | 14 +++---
.../gpu/drm/i915/gem/i915_gem_execbuffer.c| 15 ---
drivers/gpu/drm/i915/gt/intel_ring.c | 15 ---
.../gpu/drm/nouveau/nvkm/subdev/fb/ramgk104.c | 13 +++---
drivers/gpu/drm/scheduler/sched_main.c| 14 +++---
drivers/gpu/drm/ttm/ttm_bo.c | 19
drivers/gpu/drm/vmwgfx/vmwgfx_kms.c | 22 +
drivers/infiniband/hw/hfi1/tid_rdma.c | 16 ---
drivers/infiniband/hw/mlx4/main.c | 12 ++---
drivers/media/dvb-frontends/mxl5xx.c | 11 +++--
drivers/media/v4l2-core/v4l2-ctrls-api.c | 31 +++--
drivers/misc/mei/interrupt.c | 12 ++---
.../net/ethernet/qlogic/qede/qede_filter.c| 11 +++--
.../net/wireless/intel/ipw2x00/libipw_rx.c| 15 ---
drivers/power/supply/cpcap-battery.c | 11 +++--
drivers/scsi/lpfc/lpfc_bsg.c | 16 ---
drivers/staging/rtl8192e/rtl819x_TSProc.c | 17 +++
drivers/staging/rtl8192e/rtllib_rx.c | 17 ---
.../staging/rtl8192u/ieee80211/ieee80211_rx.c | 15 ---
.../rtl8192u/ieee80211/rtl819x_TSProc.c | 19
drivers/usb/gadget/composite.c| 9 ++--
fs/cifs/smb2misc.c| 10 +++--
fs/proc/kcore.c | 13 +++---
kernel/debug/kdb/kdb_main.c | 36 +--
kernel/power/snapshot.c | 10 +++--
kernel/trace/ftrace.c | 22 +
kernel/trace/trace_eprobe.c | 15 ---
kernel/trace/trace_events.c | 11 ++---
net/9p/trans_xen.c| 11 +++--
net/ipv4/udp_tunnel_nic.c | 10 +++--
net/tipc/name_table.c | 11 +++--
net/tipc/socket.c | 11 +++--
net/xfrm/xfrm_ipcomp.c| 11 +++--
sound/soc/intel/catpt/pcm.c | 13 +++---
sound/soc/sprd/sprd-mcdt.c| 13 +++---
48 files changed, 455 insertions(+), 315 deletions(-)
diff --git a/arch/arm/mach-mmp/sram.c b/arch/arm/mach-mmp/sram.c
index 6794e2db1ad5..fc47c107059b 100644
--- a/arch/arm/mach-mmp/sram.c
+++ b/arch/arm/mach-mmp/sram.c
@@ -39,19 +39,22 @@ static LIST_HEAD(sram_bank_list);
struct gen_pool *sram_get_gpool(char *pool_name)
{
struct sram_bank_info *info = NULL;
+ struct sram_bank_info *tmp;
if (!pool_name)
return NULL;
mutex_lock(_lock);
- list_for_each_entry(info, _bank_list, node)
- if (!strcmp(pool_name, info->pool_name))
+ list_for_each_entry(tmp, _bank_list, node)
+ if (!strcmp(pool_name, tmp->pool_name)) {
+ info = tmp;
break;
+ }
mutex_unlock(_lock);
- if (>node == _bank_list)
+ if (!info)
return NULL;
return info->gpool;
diff --git a/arch/arm/plat-pxa/ssp.c b/arch/arm/plat-pxa/ssp.c
index 563440315acd..4884a8c0c89b 100644
--- a/arch/arm/plat-pxa/ssp.c
+++ b/arch/arm/plat-pxa/ssp.c
@@ -38,22 +38,20 @@ static LIST_HEAD(ssp_list);
struct ssp_device *pxa_ssp_request(int port, const char *label)
{
struct ssp_device *ssp = NULL;
+ struct ssp_device *tmp;
mutex_lock(_lock);
- list_for_each_entry(ssp, _list, node) {
- if (ssp->port_id == port && ssp->use_count == 0) {
- ssp->use_count++;
- ssp->label = lab
[Intel-gfx] [PATCH 4/6] drivers: remove unnecessary use of list iterator variable
When list_for_each_entry() completes the iteration over the whole list
without breaking the loop, the iterator value will *always* be a bogus
pointer computed based on the head element.
To avoid type confusion use the actual list head directly instead of last
iterator value.
Signed-off-by: Jakob Koschel
---
drivers/dma/dw-edma/dw-edma-core.c | 4 ++--
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 3 ++-
drivers/net/wireless/ath/ath6kl/htc_mbox.c | 2 +-
3 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/drivers/dma/dw-edma/dw-edma-core.c
b/drivers/dma/dw-edma/dw-edma-core.c
index 468d1097a1ec..7883c4831857 100644
--- a/drivers/dma/dw-edma/dw-edma-core.c
+++ b/drivers/dma/dw-edma/dw-edma-core.c
@@ -136,7 +136,7 @@ static void dw_edma_free_burst(struct dw_edma_chunk *chunk)
}
/* Remove the list head */
- kfree(child);
+ kfree(chunk->burst);
chunk->burst = NULL;
}
@@ -156,7 +156,7 @@ static void dw_edma_free_chunk(struct dw_edma_desc *desc)
}
/* Remove the list head */
- kfree(child);
+ kfree(desc->chunk);
desc->chunk = NULL;
}
diff --git a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
index 091f36adbbe1..c0ea9dbc4ff6 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_ethtool.c
@@ -3963,7 +3963,8 @@ static void __i40e_reprogram_flex_pit(struct i40e_pf *pf,
* correctly, the hardware will disable flexible field parsing.
*/
if (!list_empty(flex_pit_list))
- last_offset = list_prev_entry(entry, list)->src_offset + 1;
+ last_offset = list_entry(flex_pit_list->prev,
+struct i40e_flex_pit,
list)->src_offset + 1;
for (; i < 3; i++, last_offset++) {
i40e_write_rx_ctl(>hw,
diff --git a/drivers/net/wireless/ath/ath6kl/htc_mbox.c
b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
index e3874421c4c0..cf5b05860799 100644
--- a/drivers/net/wireless/ath/ath6kl/htc_mbox.c
+++ b/drivers/net/wireless/ath/ath6kl/htc_mbox.c
@@ -104,7 +104,7 @@ static void ath6kl_credit_init(struct
ath6kl_htc_credit_info *cred_info,
* it use list_for_each_entry_reverse to walk around the whole ep list.
* Therefore assign this lowestpri_ep_dist after walk around the ep_list
*/
- cred_info->lowestpri_ep_dist = cur_ep_dist->list;
+ cred_info->lowestpri_ep_dist = *ep_list;
WARN_ON(cred_info->cur_free_credits <= 0);
--
2.25.1
