Re: [ovs-dev] [PATCH v1 1/1] ofproto: Fix NULL deref reported by Coverity.

[Ilya Maximets]

On 4/2/24 19:04, Aaron Conole wrote:
> mit...@outlook.com writes:
> 
>> From: miter 

Please, adjust the 'From' to match the Sign-off tag.

>>
>> Ofproto_class_find__() may return NULL, and dereference it to cause
>> segfault.
>>
>> Tested-by: Zhang YuHuang 
>> Signed-off-by: Lin Huang 
>> ---
> 
> I guess that type_run and type_wait aren't flagged this way because the
> only users walk the ofproto types list explicitly (so we don't get into
> this situation)?
> 
>>  ofproto/ofproto.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
>> index 122a06f30..21c6a1d82 100644
>> --- a/ofproto/ofproto.c
>> +++ b/ofproto/ofproto.c
>> @@ -800,7 +800,7 @@ ofproto_type_set_config(const char *datapath_type, const 
>> struct smap *cfg)
>>  datapath_type = ofproto_normalize_type(datapath_type);
>>  class = ofproto_class_find__(datapath_type);
>>  
>> -if (class->type_set_config) {
>> +if (class && class->type_set_config) {
> 
> This change looks good to me.  Have you a related issue or is it just
> preventative?

Hmm.  This function is only called with pre-registered types, so
this condition should not be actually possible.

The change itself is fine, but, I think, the patch name and the commit
message should be changed to indicate that this is not a real issue
and the change is made just to avoid false-positive Coverity report.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 1/1] ofproto: Fix NULL deref reported by Coverity.

[Aaron Conole]

mit...@outlook.com writes:

> From: miter 
>
> Ofproto_class_find__() may return NULL, and dereference it to cause
> segfault.
>
> Tested-by: Zhang YuHuang 
> Signed-off-by: Lin Huang 
> ---

I guess that type_run and type_wait aren't flagged this way because the
only users walk the ofproto types list explicitly (so we don't get into
this situation)?

>  ofproto/ofproto.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
> index 122a06f30..21c6a1d82 100644
> --- a/ofproto/ofproto.c
> +++ b/ofproto/ofproto.c
> @@ -800,7 +800,7 @@ ofproto_type_set_config(const char *datapath_type, const 
> struct smap *cfg)
>  datapath_type = ofproto_normalize_type(datapath_type);
>  class = ofproto_class_find__(datapath_type);
>  
> -if (class->type_set_config) {
> +if (class && class->type_set_config) {

This change looks good to me.  Have you a related issue or is it just
preventative?

>  class->type_set_config(datapath_type, cfg);
>  }
>  }

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Re: [ovs-dev] [PATCH v1 1/1] ofproto: Fix NULL deref reported by Coverity.

[0-day Robot]

Bleep bloop.  Greetings miter, I am a robot and I have tried out your patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


checkpatch:
ERROR: Author miter  needs to sign off.
WARNING: Unexpected sign-offs from developers who are not authors or co-authors 
or committers: Lin Huang 
Lines checked: 31, Warnings: 1, Errors: 1


Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

[ovs-dev] [PATCH v1 1/1] ofproto: Fix NULL deref reported by Coverity.

[miterv]

From: miter 

Ofproto_class_find__() may return NULL, and dereference it to cause
segfault.

Tested-by: Zhang YuHuang 
Signed-off-by: Lin Huang 
---
 ofproto/ofproto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ofproto/ofproto.c b/ofproto/ofproto.c
index 122a06f30..21c6a1d82 100644
--- a/ofproto/ofproto.c
+++ b/ofproto/ofproto.c
@@ -800,7 +800,7 @@ ofproto_type_set_config(const char *datapath_type, const 
struct smap *cfg)
 datapath_type = ofproto_normalize_type(datapath_type);
 class = ofproto_class_find__(datapath_type);
 
-if (class->type_set_config) {
+if (class && class->type_set_config) {
 class->type_set_config(datapath_type, cfg);
 }
 }
-- 
2.39.3

___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev