[ovs-dev] [PATCH 01/22] ovn-controller: Fix potential use-after-free in get_core_config().

[Alex Wang]

On Sun, Jul 19, 2015 at 3:44 PM, Ben Pfaff <blp at nicira.com> wrote:

> It's unsafe to hold a pointer to a row in the IDL across calls to
> ovsdb_idl_run() for that IDL.
>
> Signed-off-by: Ben Pfaff <blp at nicira.com>
> ---
>  ovn/controller/ovn-controller.c | 17 ++++++++---------
>  1 file changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/ovn/controller/ovn-controller.c
> b/ovn/controller/ovn-controller.c
> index fda1534..e571bb5 100644
> --- a/ovn/controller/ovn-controller.c
> +++ b/ovn/controller/ovn-controller.c
> @@ -95,16 +95,15 @@ get_bridge(struct controller_ctx *ctx, const char
> *name)
>  static void
>  get_core_config(struct controller_ctx *ctx)
>  {
> -    const struct ovsrec_open_vswitch *cfg;
> -
> -    cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
> -    if (!cfg) {
> -        VLOG_ERR("No Open_vSwitch row defined.");
> -        ovsdb_idl_destroy(ctx->ovs_idl);
> -        exit(EXIT_FAILURE);
> -    }
> -
>      while (1) {
> +        const struct ovsrec_open_vswitch *cfg;
> +        cfg = ovsrec_open_vswitch_first(ctx->ovs_idl);
> +        if (!cfg) {
> +            VLOG_ERR("No Open_vSwitch row defined.");
> +            ovsdb_idl_destroy(ctx->ovs_idl);
> +            exit(EXIT_FAILURE);
> +        }
> +
>

Curious, why don't you call ovsdb_idl_run(ctx->ovs_idl) first in the while
loop?  seems to me, theoretically, cfg could still get changed after
ovsdb_idl_run(ctx->ovs_idl).

Thanks,
Alex Wang,



>          const struct ovsrec_bridge *br_int;
>          const char *remote, *system_id, *br_int_name;
>
> --
> 2.1.3
>
> _______________________________________________
> dev mailing list
> dev at openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
>