diff mbox

[ovs-dev,1/7] bridge: Fix possible null pointer dereference reported by clang.

Message ID 1467176548-40488-2-git-send-email-u9012063@gmail.com
State Rejected
Headers show

Commit Message

William Tu June 29, 2016, 5:02 a.m. UTC 
Signed-off-by: William Tu <u9012063@gmail.com>
---
 vswitchd/bridge.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Ben Pfaff July 3, 2016, 4:17 a.m. UTC | #1 
On Tue, Jun 28, 2016 at 10:02:22PM -0700, William Tu wrote:
> Signed-off-by: William Tu <u9012063@gmail.com>
> ---
>  vswitchd/bridge.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
> index 8ebfc66..7244e11 100644
> --- a/vswitchd/bridge.c
> +++ b/vswitchd/bridge.c
> @@ -1999,7 +1999,7 @@ find_local_hw_addr(const struct bridge *br, struct eth_addr *ea,
>  
>              /* The local port doesn't count (since we're trying to choose its
>               * MAC address anyway). */
> -            if (iface->ofp_port == OFPP_LOCAL) {
> +            if (iface && iface->ofp_port == OFPP_LOCAL) {
>                  continue;
>              }

A port is guaranteed to have at least one interface, so 'iface' can't be
null here.  Even if it could, this patch wouldn't help because a few
lines down there's an unconditional dereference.

I'd happily add an assertion to document this, e.g.:
        http://openvswitch.org/pipermail/dev/2016-July/074352.html
William Tu July 5, 2016, 1:54 p.m. UTC | #2 
Hi Ben,

Thanks, I tested your patch below and clang no longer reports no error.

Regards,
William

> A port is guaranteed to have at least one interface, so 'iface' can't be
> null here.  Even if it could, this patch wouldn't help because a few
> lines down there's an unconditional dereference.
>
> I'd happily add an assertion to document this, e.g.:
>         http://openvswitch.org/pipermail/dev/2016-July/074352.html
Ben Pfaff July 5, 2016, 3:30 p.m. UTC | #3 
Thanks, applied to master.

On Tue, Jul 05, 2016 at 06:54:41AM -0700, William Tu wrote:
> Hi Ben,
> 
> Thanks, I tested your patch below and clang no longer reports no error.
> 
> Regards,
> William
> 
> > A port is guaranteed to have at least one interface, so 'iface' can't be
> > null here.  Even if it could, this patch wouldn't help because a few
> > lines down there's an unconditional dereference.
> >
> > I'd happily add an assertion to document this, e.g.:
> >         http://openvswitch.org/pipermail/dev/2016-July/074352.html
diff mbox

Patch

diff --git a/vswitchd/bridge.c b/vswitchd/bridge.c
index 8ebfc66..7244e11 100644
--- a/vswitchd/bridge.c
+++ b/vswitchd/bridge.c
@@ -1999,7 +1999,7 @@  find_local_hw_addr(const struct bridge *br, struct eth_addr *ea,
 
             /* The local port doesn't count (since we're trying to choose its
              * MAC address anyway). */
-            if (iface->ofp_port == OFPP_LOCAL) {
+            if (iface && iface->ofp_port == OFPP_LOCAL) {
                 continue;
             }