* Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() [not found] ` <e8620e70-16bb-639c-9446-68dcd199253f@linux.ibm.com> @ 2022-04-28 9:46 ` Christophe Marie Francois Dupont de Dinechin 2022-04-28 9:51 ` Christophe Marie Francois Dupont de Dinechin 0 siblings, 1 reply; 6+ messages in thread From: Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 9:46 UTC (permalink / raw) To: muriloo Cc: Michael S. Tsirkin, linux-kernel, virtualization, mopsfelder, Christophe de Dinechin > On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > > On 4/14/22 23:30, Murilo Opsfelder Araujo wrote: >> GCC 12 enhanced -Waddress when comparing array address to null [0], >> which warns: >> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’: >> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must not be NULL [-Waddress] >> 257 | if (vp_dev->msix_affinity_masks[i]) >> | ^~~~~~ >> In fact, the verification is comparing the result of a pointer >> arithmetic, the address "msix_affinity_masks + i", which will always >> evaluate to true. >> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass >> NULL, not requiring non-null verification. So remove the verification >> to make compiler happy (happy compiler, happy life). >> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103 >> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> >> --- >> drivers/virtio/virtio_pci_common.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >> index d724f676608b..5046efcffb4c 100644 >> --- a/drivers/virtio/virtio_pci_common.c >> +++ b/drivers/virtio/virtio_pci_common.c >> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev) >> if (vp_dev->msix_affinity_masks) { >> for (i = 0; i < vp_dev->msix_vectors; i++) >> - if (vp_dev->msix_affinity_masks[i]) >> - free_cpumask_var(vp_dev->msix_affinity_masks[i]); >> + free_cpumask_var(vp_dev->msix_affinity_masks[i]); >> } >> if (vp_dev->msix_enabled) { > > After I sent this message, I realized that Christophe (copied here) > had already proposed a fix: > > https://lore.kernel.org/lkml/20220414150855.2407137-4-dinechin@redhat.com/ > > Christophe, > > Since free_cpumask_var() calls kfree() and kfree() is null-safe, > can we just drop this null verification and call free_cpumask_var() right away? Apologies for the delay in responding, broken laptop… In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have: typedef struct cpumask cpumask_var_t[1]; So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the warning) but also a static pointer, so not kfree-safe IMO. _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() 2022-04-28 9:46 ` [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 9:51 ` Christophe Marie Francois Dupont de Dinechin 2022-04-28 9:55 ` Christophe Marie Francois Dupont de Dinechin 0 siblings, 1 reply; 6+ messages in thread From: Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 9:51 UTC (permalink / raw) To: muriloo Cc: Michael S. Tsirkin, linux-kernel, virtualization, mopsfelder, Christophe de Dinechin > On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: > > > >> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: >> >> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote: >>> GCC 12 enhanced -Waddress when comparing array address to null [0], >>> which warns: >>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’: >>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must not be NULL [-Waddress] >>> 257 | if (vp_dev->msix_affinity_masks[i]) >>> | ^~~~~~ >>> In fact, the verification is comparing the result of a pointer >>> arithmetic, the address "msix_affinity_masks + i", which will always >>> evaluate to true. >>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass >>> NULL, not requiring non-null verification. So remove the verification >>> to make compiler happy (happy compiler, happy life). >>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103 >>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> >>> --- >>> drivers/virtio/virtio_pci_common.c | 3 +-- >>> 1 file changed, 1 insertion(+), 2 deletions(-) >>> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >>> index d724f676608b..5046efcffb4c 100644 >>> --- a/drivers/virtio/virtio_pci_common.c >>> +++ b/drivers/virtio/virtio_pci_common.c >>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev) >>> if (vp_dev->msix_affinity_masks) { >>> for (i = 0; i < vp_dev->msix_vectors; i++) >>> - if (vp_dev->msix_affinity_masks[i]) >>> - free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>> + free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>> } >>> if (vp_dev->msix_enabled) { >> >> After I sent this message, I realized that Christophe (copied here) >> had already proposed a fix: >> >> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinechin@redhat.com/ >> >> Christophe, >> >> Since free_cpumask_var() calls kfree() and kfree() is null-safe, >> can we just drop this null verification and call free_cpumask_var() right away? > > Apologies for the delay in responding, broken laptop… > > In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have: > > typedef struct cpumask cpumask_var_t[1]; > > So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the warning) > but also a static pointer, so not kfree-safe IMO. … which also renders my own patch invalid :-/ Compiler warnings are good. Clearly not sufficient. _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() 2022-04-28 9:51 ` Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 9:55 ` Christophe Marie Francois Dupont de Dinechin 2022-04-28 11:03 ` Michael S. Tsirkin 0 siblings, 1 reply; 6+ messages in thread From: Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 9:55 UTC (permalink / raw) To: Murilo Opsfelder Araujo Cc: Michael S. Tsirkin, linux-kernel, virtualization, mopsfelder, Christophe de Dinechin > On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: > > > >> On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: >> >> >> >>> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: >>> >>> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote: >>>> GCC 12 enhanced -Waddress when comparing array address to null [0], >>>> which warns: >>>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’: >>>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must not be NULL [-Waddress] >>>> 257 | if (vp_dev->msix_affinity_masks[i]) >>>> | ^~~~~~ >>>> In fact, the verification is comparing the result of a pointer >>>> arithmetic, the address "msix_affinity_masks + i", which will always >>>> evaluate to true. >>>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass >>>> NULL, not requiring non-null verification. So remove the verification >>>> to make compiler happy (happy compiler, happy life). >>>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103 >>>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> >>>> --- >>>> drivers/virtio/virtio_pci_common.c | 3 +-- >>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >>>> index d724f676608b..5046efcffb4c 100644 >>>> --- a/drivers/virtio/virtio_pci_common.c >>>> +++ b/drivers/virtio/virtio_pci_common.c >>>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev) >>>> if (vp_dev->msix_affinity_masks) { >>>> for (i = 0; i < vp_dev->msix_vectors; i++) >>>> - if (vp_dev->msix_affinity_masks[i]) >>>> - free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>>> + free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>>> } >>>> if (vp_dev->msix_enabled) { >>> >>> After I sent this message, I realized that Christophe (copied here) >>> had already proposed a fix: >>> >>> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinechin@redhat.com/ >>> >>> Christophe, >>> >>> Since free_cpumask_var() calls kfree() and kfree() is null-safe, >>> can we just drop this null verification and call free_cpumask_var() right away? >> >> Apologies for the delay in responding, broken laptop… >> >> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have: >> >> typedef struct cpumask cpumask_var_t[1]; >> >> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the warning) >> but also a static pointer, so not kfree-safe IMO. > > … which also renders my own patch invalid :-/ > > Compiler warnings are good. Clearly not sufficient. Ah, I just noticed that free_cpumask_var is a noop in that case. So yes, your fix is better :-) _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() 2022-04-28 9:55 ` Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 11:03 ` Michael S. Tsirkin 2022-04-28 11:43 ` Christophe Marie Francois Dupont de Dinechin 2022-04-28 11:52 ` Christophe Marie Francois Dupont de Dinechin 0 siblings, 2 replies; 6+ messages in thread From: Michael S. Tsirkin @ 2022-04-28 11:03 UTC (permalink / raw) To: Christophe Marie Francois Dupont de Dinechin Cc: linux-kernel, virtualization, mopsfelder, Murilo Opsfelder Araujo, Christophe de Dinechin On Thu, Apr 28, 2022 at 11:55:31AM +0200, Christophe Marie Francois Dupont de Dinechin wrote: > > > > On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: > > > > > > > >> On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: > >> > >> > >> > >>> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: > >>> > >>> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote: > >>>> GCC 12 enhanced -Waddress when comparing array address to null [0], > >>>> which warns: > >>>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’: > >>>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must not be NULL [-Waddress] > >>>> 257 | if (vp_dev->msix_affinity_masks[i]) > >>>> | ^~~~~~ > >>>> In fact, the verification is comparing the result of a pointer > >>>> arithmetic, the address "msix_affinity_masks + i", which will always > >>>> evaluate to true. > >>>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass > >>>> NULL, not requiring non-null verification. So remove the verification > >>>> to make compiler happy (happy compiler, happy life). > >>>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103 > >>>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> > >>>> --- > >>>> drivers/virtio/virtio_pci_common.c | 3 +-- > >>>> 1 file changed, 1 insertion(+), 2 deletions(-) > >>>> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c > >>>> index d724f676608b..5046efcffb4c 100644 > >>>> --- a/drivers/virtio/virtio_pci_common.c > >>>> +++ b/drivers/virtio/virtio_pci_common.c > >>>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev) > >>>> if (vp_dev->msix_affinity_masks) { > >>>> for (i = 0; i < vp_dev->msix_vectors; i++) > >>>> - if (vp_dev->msix_affinity_masks[i]) > >>>> - free_cpumask_var(vp_dev->msix_affinity_masks[i]); > >>>> + free_cpumask_var(vp_dev->msix_affinity_masks[i]); > >>>> } > >>>> if (vp_dev->msix_enabled) { > >>> > >>> After I sent this message, I realized that Christophe (copied here) > >>> had already proposed a fix: > >>> > >>> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinechin@redhat.com/ > >>> > >>> Christophe, > >>> > >>> Since free_cpumask_var() calls kfree() and kfree() is null-safe, > >>> can we just drop this null verification and call free_cpumask_var() right away? > >> > >> Apologies for the delay in responding, broken laptop… > >> > >> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have: > >> > >> typedef struct cpumask cpumask_var_t[1]; > >> > >> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the warning) > >> but also a static pointer, so not kfree-safe IMO. > > > > … which also renders my own patch invalid :-/ > > > > Compiler warnings are good. Clearly not sufficient. > > Ah, I just noticed that free_cpumask_var is a noop in that case. > > So yes, your fix is better :-) ACK then? _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() 2022-04-28 11:03 ` Michael S. Tsirkin @ 2022-04-28 11:43 ` Christophe Marie Francois Dupont de Dinechin 2022-04-28 11:52 ` Christophe Marie Francois Dupont de Dinechin 1 sibling, 0 replies; 6+ messages in thread From: Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 11:43 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-kernel, virtualization, mopsfelder, Murilo Opsfelder Araujo, Christophe de Dinechin [-- Attachment #1.1: Type: text/plain, Size: 3460 bytes --] > On 28 Apr 2022, at 13:03, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Thu, Apr 28, 2022 at 11:55:31AM +0200, Christophe Marie Francois Dupont de Dinechin wrote: >> >> >>> On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: >>> >>> >>> >>>> On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: >>>> >>>> >>>> >>>>> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: >>>>> >>>>> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote: >>>>>> GCC 12 enhanced -Waddress when comparing array address to null [0], >>>>>> which warns: >>>>>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’: >>>>>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must not be NULL [-Waddress] >>>>>> 257 | if (vp_dev->msix_affinity_masks[i]) >>>>>> | ^~~~~~ >>>>>> In fact, the verification is comparing the result of a pointer >>>>>> arithmetic, the address "msix_affinity_masks + i", which will always >>>>>> evaluate to true. >>>>>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass >>>>>> NULL, not requiring non-null verification. So remove the verification >>>>>> to make compiler happy (happy compiler, happy life). >>>>>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103 >>>>>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> >>>>>> --- >>>>>> drivers/virtio/virtio_pci_common.c | 3 +-- >>>>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>>>> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >>>>>> index d724f676608b..5046efcffb4c 100644 >>>>>> --- a/drivers/virtio/virtio_pci_common.c >>>>>> +++ b/drivers/virtio/virtio_pci_common.c >>>>>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev) >>>>>> if (vp_dev->msix_affinity_masks) { >>>>>> for (i = 0; i < vp_dev->msix_vectors; i++) >>>>>> - if (vp_dev->msix_affinity_masks[i]) >>>>>> - free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>>>>> + free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>>>>> } >>>>>> if (vp_dev->msix_enabled) { >>>>> >>>>> After I sent this message, I realized that Christophe (copied here) >>>>> had already proposed a fix: >>>>> >>>>> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinechin@redhat.com/ >>>>> >>>>> Christophe, >>>>> >>>>> Since free_cpumask_var() calls kfree() and kfree() is null-safe, >>>>> can we just drop this null verification and call free_cpumask_var() right away? >>>> >>>> Apologies for the delay in responding, broken laptop… >>>> >>>> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have: >>>> >>>> typedef struct cpumask cpumask_var_t[1]; >>>> >>>> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the warning) >>>> but also a static pointer, so not kfree-safe IMO. >>> >>> … which also renders my own patch invalid :-/ >>> >>> Compiler warnings are good. Clearly not sufficient. >> >> Ah, I just noticed that free_cpumask_var is a noop in that case. >> >> So yes, your fix is better :-) > > ACK then? Yes. Acked-by: Christophe de Dinechin <dinechin@redhat.com <mailto:dinechin@redhat.com>> > [-- Attachment #1.2: Type: text/html, Size: 6100 bytes --] [-- Attachment #2: Type: text/plain, Size: 183 bytes --] _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() 2022-04-28 11:03 ` Michael S. Tsirkin 2022-04-28 11:43 ` Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 11:52 ` Christophe Marie Francois Dupont de Dinechin 1 sibling, 0 replies; 6+ messages in thread From: Christophe Marie Francois Dupont de Dinechin @ 2022-04-28 11:52 UTC (permalink / raw) To: Michael S. Tsirkin Cc: linux-kernel, virtualization, mopsfelder, Murilo Opsfelder Araujo, Christophe de Dinechin [Resend, still struggling with new laptop email settings] > On 28 Apr 2022, at 13:03, Michael S. Tsirkin <mst@redhat.com> wrote: > > On Thu, Apr 28, 2022 at 11:55:31AM +0200, Christophe Marie Francois Dupont de Dinechin wrote: >> >> >>> On 28 Apr 2022, at 11:51, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: >>> >>> >>> >>>> On 28 Apr 2022, at 11:46, Christophe Marie Francois Dupont de Dinechin <cdupontd@redhat.com> wrote: >>>> >>>> >>>> >>>>> On 15 Apr 2022, at 05:51, Murilo Opsfelder Araújo <muriloo@linux.ibm.com> wrote: >>>>> >>>>> On 4/14/22 23:30, Murilo Opsfelder Araujo wrote: >>>>>> GCC 12 enhanced -Waddress when comparing array address to null [0], >>>>>> which warns: >>>>>> drivers/virtio/virtio_pci_common.c: In function ‘vp_del_vqs’: >>>>>> drivers/virtio/virtio_pci_common.c:257:29: warning: the comparison will always evaluate as ‘true’ for the pointer operand in ‘vp_dev->msix_affinity_masks + (sizetype)((long unsigned int)i * 256)’ must not be NULL [-Waddress] >>>>>> 257 | if (vp_dev->msix_affinity_masks[i]) >>>>>> | ^~~~~~ >>>>>> In fact, the verification is comparing the result of a pointer >>>>>> arithmetic, the address "msix_affinity_masks + i", which will always >>>>>> evaluate to true. >>>>>> Under the hood, free_cpumask_var() calls kfree(), which is safe to pass >>>>>> NULL, not requiring non-null verification. So remove the verification >>>>>> to make compiler happy (happy compiler, happy life). >>>>>> [0] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102103 >>>>>> Signed-off-by: Murilo Opsfelder Araujo <muriloo@linux.ibm.com> >>>>>> --- >>>>>> drivers/virtio/virtio_pci_common.c | 3 +-- >>>>>> 1 file changed, 1 insertion(+), 2 deletions(-) >>>>>> diff --git a/drivers/virtio/virtio_pci_common.c b/drivers/virtio/virtio_pci_common.c >>>>>> index d724f676608b..5046efcffb4c 100644 >>>>>> --- a/drivers/virtio/virtio_pci_common.c >>>>>> +++ b/drivers/virtio/virtio_pci_common.c >>>>>> @@ -254,8 +254,7 @@ void vp_del_vqs(struct virtio_device *vdev) >>>>>> if (vp_dev->msix_affinity_masks) { >>>>>> for (i = 0; i < vp_dev->msix_vectors; i++) >>>>>> - if (vp_dev->msix_affinity_masks[i]) >>>>>> - free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>>>>> + free_cpumask_var(vp_dev->msix_affinity_masks[i]); >>>>>> } >>>>>> if (vp_dev->msix_enabled) { >>>>> >>>>> After I sent this message, I realized that Christophe (copied here) >>>>> had already proposed a fix: >>>>> >>>>> https://lore.kernel.org/lkml/20220414150855.2407137-4-dinechin@redhat.com/ >>>>> >>>>> Christophe, >>>>> >>>>> Since free_cpumask_var() calls kfree() and kfree() is null-safe, >>>>> can we just drop this null verification and call free_cpumask_var() right away? >>>> >>>> Apologies for the delay in responding, broken laptop… >>>> >>>> In the case where CONFIG_CPUMASK_OFFSTACK is not defined, we have: >>>> >>>> typedef struct cpumask cpumask_var_t[1]; >>>> >>>> So that vp_dev->msix_affinity_masks[i] is statically not null (that’s the warning) >>>> but also a static pointer, so not kfree-safe IMO. >>> >>> … which also renders my own patch invalid :-/ >>> >>> Compiler warnings are good. Clearly not sufficient. >> >> Ah, I just noticed that free_cpumask_var is a noop in that case. >> >> So yes, your fix is better :-) > > ACK then? Yes. Acked-by: Christophe de Dinechin <dinechin@redhat.com> > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-28 11:53 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20220415023002.49805-1-muriloo@linux.ibm.com>
[not found] ` <e8620e70-16bb-639c-9446-68dcd199253f@linux.ibm.com>
2022-04-28 9:46 ` [PATCH] virtio-pci: Remove wrong address verification in vp_del_vqs() Christophe Marie Francois Dupont de Dinechin
2022-04-28 9:51 ` Christophe Marie Francois Dupont de Dinechin
2022-04-28 9:55 ` Christophe Marie Francois Dupont de Dinechin
2022-04-28 11:03 ` Michael S. Tsirkin
2022-04-28 11:43 ` Christophe Marie Francois Dupont de Dinechin
2022-04-28 11:52 ` Christophe Marie Francois Dupont de Dinechin
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).