fault and when we do that the performance hit for
setting index to zero will go away for most people. (Zeroing the stack
makes uninitialized variable bugs defined behavior and has security
benefits).
311 else
312 return false;
313
314 return true;
315 }
regards,
dan carpenter
patch. We generally say "fix the checker and don't
silence the warning" but in this case I feel like the checker is doing
the best possible thing and I'm not going to fix it. Trying to silence
this warning in Smatch would come with some real downsides.
regards,
dan carpenter
D_MMAPBATCH_V2 ioctl")
Signed-off-by: Dan Carpenter
---
drivers/xen/privcmd.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/xen/privcmd.c b/drivers/xen/privcmd.c
index ad17166b0ef6..1e59b76c618e 100644
--- a/drivers/xen/privcmd.c
+++ b/drivers/xen/privcmd.c
@@ -45
On Fri, Jul 15, 2022 at 08:56:30AM +, Oleksandr Tyshchenko wrote:
>
> On 15.07.22 11:20, Dan Carpenter wrote:
>
>
> Hello Dan
>
> > The "m.num * sizeof(*m.arr)" multiplication can have an integer overflow
> > on 32 bit systems. Probably no
checkpatch complains about. Also
unsigned type would break if "len" were not capped at MAX_RW_COUNT. Use
size_t for the min(). (No effect on runtime for the min_t() change).
Fixes: 2fb3683e7b16 ("xen: Add xenbus device driver")
Signed-off-by: Dan Carpenter
---
drivers/xen/xe
The change from kcalloc() to kvmalloc() means that arg->nr_pages
might now be large enough that the "args->nr_pages << PAGE_SHIFT" can
result in an integer overflow.
Fixes: b3f7931f5c61 ("xen/gntdev: switch from kcalloc() to kvcalloc()")
Signed-off-by: Dan C
array or outside of it (note the "<" used for the
> >>> test).
> >> Thank you for your explanation.
> > This looks like a smatch bug. I'd feed it back upstream.
>
> +Dan
>
Yep. Thanks for the bug report. Let me test my fix and push it later
this week.
Btw, it might help readability slightly if we made it more clear we were
doing pointer math:
*addr >= (void *)&early_idt_handler_array[0] &&
*addr < (void
*)&early_idt_handler_array[NUM_EXCEPTION_VECTORS]) {
nr = (*addr - (void *)&early_idt_handler_array[0]) /
Regardless, this is definitely a bug in Smatch and I will push a fix.
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
If we really have other clear rules, then it should be encoded into
get_maintainer.pl so that it's automatic.
My other question is why do the linux-arm-ker...@lists.infradead.org
people feel like they need to be CC'd about every driver??? I always
remove them from the CC list unless it'
70 return 0;
371
372 out:
--> 373 free_active_ring(map);
^^^
Unchecked dereference. This style of error handling tends to have bugs.
https://plus.google.com/u/0/106378716002406849458/posts/1Ud9JbaYnPr
374 retur
On Wed, Mar 28, 2018 at 01:57:20PM +0200, Juergen Gross wrote:
> On 28/03/18 13:47, Dan Carpenter wrote:
> > If acpi_id is == nr_acpi_bits, then we access one element beyond the end
> > of the acpi_psd[] array or we set one bit beyond the end of the bit map
> > when we
visor.")
Signed-off-by: Dan Carpenter
Reviewed-by: Joao Martins
Reviewed-by: Juergen Gross
diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
index c80195e8fbd1..b29f4e40851f 100644
--- a/drivers/xen/xen-acpi-processor.c
+++ b/drivers/xen/xen-acpi-processor.c
drm_dev_alloc() returns error pointers, it never returns NULL.
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/xen/xen_drm_front.c
b/drivers/gpu/drm/xen/xen_drm_front.c
index 1b0ea9ac330e..86
The xen_drm_front_shbuf_alloc() function was returning a mix of error
pointers and NULL and the the caller wasn't checking correctly. I've
changed it to always return error pointer consistently.
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend"
If the loop times out then we want to exit with "to" set to zero, but in
the current code it's set to -1.
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend")
Signed-off-by: Dan Carpenter
diff --git a/drivers/gpu/drm/xen/xen_drm_front
kfree() doesn't accept error pointers so I've set "str" to NULL on these
paths.
Fixes: fd3b36045c2c ("ALSA: xen-front: Read sound driver configuration from Xen
store")
Signed-off-by: Dan Carpenter
diff --git a/sound/xen/xen_snd_front_cfg.c b/sound/xen/xen_snd_
tualized sound
frontend driver")
Signed-off-by: Dan Carpenter
diff --git a/sound/xen/xen_snd_front.c b/sound/xen/xen_snd_front.c
index c18973a9bc9b..b089b13b5160 100644
--- a/sound/xen/xen_snd_front.c
+++ b/sound/xen/xen_snd_front.c
@@ -334,7 +334,7 @@ static int xen_drv_remove(struct
"sndif_format" needs to be signed for the error handling to work.
Fixes: 1cee559351a7 ("ALSA: xen-front: Implement ALSA virtual sound driver")
Signed-off-by: Dan Carpenter
diff --git a/sound/xen/xen_snd_front_alsa.c b/sound/xen/xen_snd_front_alsa.c
index 5041f83e98d2..43478
843 } else
844 rc = 0;
845 }
846
847 out:
848 up_write(&mm->mmap_sem);
849 kfree(pfns);
850
851 return rc;
852 }
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
If dma_get_sgtable() fails then we return NULL.
Fix that and it should be good.
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Thu, Jul 12, 2018 at 02:58:00PM +0300, Oleksandr Andrushchenko wrote:
> On 06/18/2018 03:32 PM, Oleksandr Andrushchenko wrote:
> > On 06/18/2018 03:29 PM, Dan Carpenter wrote:
> > > On Mon, Jun 18, 2018 at 09:07:09AM +0300, Oleksandr Andrushchenko wrote:
>
The xen_drm_front_gem_get_sg_table() function is supposed to return
error pointer. The current code, would trigger a NULL dereference in
drm_gem_map_dma_buf().
Fixes: c575b7eeb89f ("drm/xen-front: Add support for Xen PV display frontend")
Signed-off-by: Dan Carpenter
diff --git a/d
ese earlier. After a while these
things all look the same. I once accidentally wrote and sent the same
patch three times without realizing.
Yeah. Please resend a v3.
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists
Oleksandr sent this patch already. Please disregard mine.
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
Reviewed-by: Dan Carpenter
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
of a warning.
>
> FWIW, this series has found at least one bug so far:
> https://lore.kernel.org/lkml/CAFCwf11izHF=g1mGry1fE5kvFFFrxzhPSM6qKAO8gxSp=kr...@mail.gmail.com/
This is a fallthrough to a return and not to a break. That should
trigger a warning. The fallthrough to a break should not generate a
warning.
The bug we're trying to fix is "missing break statement" but if the
result of the bug is "we hit a break statement" then now we're just
talking about style. GCC should limit itself to warning about
potentially buggy code.
regards,
dan carpenter
h (x) {
> case 0:
> ++x;
> default:
> ;
> }
Don't warn for this.
If adding a break statement changes the flow of the code then warn about
potentially missing break statements, but if it doesn't change anything
then don't warn about it.
regards,
dan carpenter
ping functions like xenbus_write()
and we could be holding a spinlock here.
pvcalls_front_connect() <-- holds &bedata->socket_lock
--> create_active()
--> xenbus_alloc_evtchn()
431 else
432 *port = alloc_unbound.port;
433
434 return err;
435 }
regards,
dan carpenter
out_free_vm_area;
113 return 0;
114 out_free_vm_area:
115 free_vm_area(area->area);
116 out_free_ptes:
117 kfree(area->ptes);
^
This frees a different pointer from what was allocated.
118 return -ENOMEM;
119 }
regards,
dan carpenter
idx].handle;
840 } else {
841 continue;
842 }
843 if (use_persistent_gnts &&
regards,
dan carpenter
When there is an error the caller frees "info->node" so the free here
will result in a double free. We should just delete first kfree().
Fixes: 3848e4e0a32a ("xen/xenbus: avoid large structs and arrays on the stack")
Signed-off-by: Dan Carpenter
---
drivers/xen/xen
Looks great! Thanks.
Reviewed-by: Dan Carpenter
regards,
dan carpenter
this and the rest of misused places with IS_ERR_OR_NULL in the
> >> driver.
> >>
> >> Fixes: c575b7eeb89f: "drm/xen-front: Add support for Xen PV display
> >> frontend"
> >
> > Again forgot to Cc stable?
>
> I was just not sure if these minor fixes need to go the stable, but I will add
Correct. It's still a bug because it's setting the error code
incorrectly on the impossible path. But fortunately impossible things
don't affect runtime.
regards,
dan carpenter
ers for stream with index %d\n",
498 stream->index);
499 return ret;
500 }
regards,
dan carpenter
ext because users will be doing
"echo 1234 > /sys/foo".
> If you are worried about overflow you need a range check
> before the multiply.
This is probably a case where if the users cause an integer overflow
then they get what they deserve.
regards,
dan carpenter
Smatch complains that "len" is uninitialized if xenbus_read() fails so
let's add some error handling.
Signed-off-by: Dan Carpenter
diff --git a/drivers/xen/pvcalls-front.c b/drivers/xen/pvcalls-front.c
index 40caa92bff33..afa3f1b5d807 100644
--- a/drivers/xen/pvcalls-front.c
++
bedata->ref can't be less than zero because it's unsigned. This affects
certain error paths in probe. We first set ->ref = -1 and then we set
it to a valid value later.
Fixes: 219681909913 ("xen/pvcalls: connect to the backend")
Signed-off-by: Dan Carpenter
diff -
switch (cmd) {
> + case EVTCHNOP_bind_interdomain:
> + len = sizeof(struct evtchn_bind_interdomain);
> + break;
This was in the original code, but I'm slightly surpprised that we're
using a switch statement here instead of a table. I would have thought
this is a fast path but I don't know xen at all.
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
On Fri, Feb 02, 2018 at 05:11:02PM +0100, Arnd Bergmann wrote:
> On Fri, Feb 2, 2018 at 4:53 PM, Dan Carpenter
> wrote:
> > On Fri, Feb 02, 2018 at 04:32:31PM +0100, Arnd Bergmann wrote:
> >> switch (cmd) {
> >> + case EVTCHNOP_bind_interdomain:
> >
visor.")
Signed-off-by: Dan Carpenter
diff --git a/drivers/xen/xen-acpi-processor.c b/drivers/xen/xen-acpi-processor.c
index c80195e8fbd1..d23c9c150199 100644
--- a/drivers/xen/xen-acpi-processor.c
+++ b/drivers/xen/xen-acpi-processor.c
@@ -364,7 +364,7 @@ read_acpi_id(acpi_handle handle, u32
. I remember I reported some a while back but never
heard back. https://lore.kernel.org/all/20210802144037.GA29540@kili/
regards,
dan carpenter
arch/x86/xen/p2m.c:189 alloc_p2m_page() warn: sleeping in atomic context
xen_create_contiguous_region() <- disables preempt
xen_destroy_contiguous_reg
roduce a blank line here. Checkpatch will complain if you
have to blank lines in a row. It won't complain about the patch but it
will complain if you apply the patch and then re-run checkpatch -f on
the file. (I didn't test this but it's wrong either way. :P).
regards,
dan carpenter
(err) {
750 xenbus_dev_error(info->dev, err,
751 "%s: writing dev_state_path", __func__);
752 return err;
753 }
754 }
755
756 return 0;
757 }
regards,
dan carpenter
'm not a Xen maintainer so I can't really comment on their style
choices. However, as one of the kernel-janitors list people, I would
say that not everyone agrees with Markus's style preferences. Markus
was banned from the list for a while, but we unbanned everyone when we
transitioned to the new list infrastructure. Do a search on lore to
find out more. https://lore.kernel.org/all/?q=Elfring
Perhaps wait for feedback from the maintainers for how to proceed?
regards,
dan carpenter
(Debian 11.3.0-8) 11.3.0
If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot
| Reported-by: Dan Carpenter
| Link: https://lore.kernel.org/r/202303112149.xd47qkoy-...@intel.com/
smatch warnings:
drivers/pnp/quirks.c:248 quirk_system_pci_resources() warn
The error code needs to be set on these error paths.
Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()")
Fixes: d2ba3166f23b ("xen/events: move drivers/xen/events.c into
drivers/xen/events/")
Signed-off-by: Dan Carpenter
---
Are we going to backport these
On Mon, Nov 27, 2023 at 02:17:05PM +0100, Juergen Gross wrote:
> On 27.11.23 13:57, Dan Carpenter wrote:
> > The error code needs to be set on these error paths.
> >
> > Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()")
> > Fixes: d2ba31
Return -ENOMEM if xen_irq_init() fails. currently the code returns an
uninitialized variable or zero.
Fixes: 5dd9ad32d775 ("xen/events: drop xen_allocate_irqs_dynamic()")
Signed-off-by: Dan Carpenter
---
v2: Part of v1 was bogus. Delete that bit.
drivers/xen/events/events_base.c |
the sock, and don't free it in the caller.
Fixes: 5db4d286a8ef ("xen/pvcalls: implement connect command")
Signed-off-by: Dan Carpenter
---
drivers/xen/pvcalls-back.c | 9 -
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/drivers/xen/pvcalls-back.c b/driv
> args)
> if (is_sign)
> val.s = simple_strntoll(str,
> field_width >= 0 ? field_width
> : INT_MAX,
> - &next, base);
> + &next, base, &overflow);
> else
> val.u = simple_strntoull(str,
>field_width >= 0 ? field_width
> : INT_MAX,
> - &next, base);
> + &next, base, &overflow);
> + if (unlikely(overflow && !allow_overflow))
So that means that *overflow can be uninitialized here.
> + break;
>
> switch (qualifier) {
regards,
dan carpenter
r error pointers
but that's because of a bug in the unreleased version of Smatch and
because gem_create() uses IS_ERR_OR_NULL().
141
142 return &xen_obj->base;
143 }
regards,
dan carpenter
g, because if the pointer was really NULL then it's
not handled correctly and would eventually lead to a runtime failure.
Normally Smatch is smart enough to know that the pointer isn't NULL so
it doesn't generate a warning but yesterday I introduced a bug in Smatch
by mistake. I
On Tue, Apr 21, 2020 at 05:29:02PM +0200, Julia Lawall wrote:
>
>
> On Tue, 21 Apr 2020, Dan Carpenter wrote:
>
> > Hi Kernel Janitors,
> >
> > Here is another idea that someone could work on, fixing the
> > IS_ERR_OR_NULL() checks in the xen driver.
>
On Tue, Apr 21, 2020 at 08:59:09PM +0200, Julia Lawall wrote:
>
>
> On Tue, 21 Apr 2020, Dan Carpenter wrote:
>
> > On Tue, Apr 21, 2020 at 05:29:02PM +0200, Julia Lawall wrote:
> > >
> > >
> > > On Tue, 21 Apr 2020, Dan Carpenter wrote:
> > &g
The "cpu" variable comes from the sscanf() so Smatch marks it as
untrusted data. We can't pass a higher value than "nr_cpu_ids" to
cpu_possible() or it results in an out of bounds access.
Fixes: d68d82afd4c8 ("xen: implement CPU hotplugging")
Signed-off-
ller")
Signed-off-by: Dan Carpenter
---
arch/x86/include/asm/xen/hypercall.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/x86/include/asm/xen/hypercall.h
b/arch/x86/include/asm/xen/hypercall.h
index de6f0d59a24f..2863c2026655 100644
--- a/arch/x86/include/asm/xen/hypercall.h
+++ b
c:753 common_process() warn: passing casted
pointer '&sh_info->flags' to 'test_bit()' 32 vs 64.
drivers/xen/xen-pciback/pci_stub.c:799 xen_pcibk_slot_reset() warn: passing
casted pointer '&psdev->pdev->sh_info->flags' to 'test_bit()' 32 vs 64.
drivers/xen/xen-pciback/pci_stub.c:857 xen_pcibk_mmio_enabled() warn: passing
casted pointer '&psdev->pdev->sh_info->flags' to 'test_bit()' 32 vs 64.
drivers/xen/xen-pciback/pci_stub.c:916 xen_pcibk_error_detected() warn: passing
casted pointer '&psdev->pdev->sh_info->flags' to 'test_bit()' 32 vs 64.
drivers/xen/xen-pciback/pci_stub.c:969 xen_pcibk_error_resume() warn: passing
casted pointer '&psdev->pdev->sh_info->flags' to 'test_bit()' 32 vs 64.
regards,
dan carpenter
___
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
56 matches
Mail list logo
