From patchwork Tue Sep 6 01:45:53 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sam Mendoza-Jonas X-Patchwork-Id: 666119 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from lists.ozlabs.org (lists.ozlabs.org [103.22.144.68]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3sSqFW41Zyz9s5g for ; Tue, 6 Sep 2016 11:46:23 +1000 (AEST) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=mendozajonas.com header.i=@mendozajonas.com header.b=IA8JZO2z; dkim-atps=neutral Received: from ozlabs.org (lists.ozlabs.org [IPv6:2401:3900:2:1::3]) by lists.ozlabs.org (Postfix) with ESMTP id 3sSqFW2pHkzDrpj for ; Tue, 6 Sep 2016 11:46:23 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=fail reason="signature verification failed" (1024-bit key; unprotected) header.d=mendozajonas.com header.i=@mendozajonas.com header.b=IA8JZO2z; dkim-atps=neutral X-Original-To: petitboot@lists.ozlabs.org Delivered-To: petitboot@lists.ozlabs.org Received: from mendozajonas.com (mendozajonas.com [188.166.185.233]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by lists.ozlabs.org (Postfix) with ESMTPS id 3sSqFF1RQ1zDrpj for ; Tue, 6 Sep 2016 11:46:09 +1000 (AEST) Authentication-Results: lists.ozlabs.org; dkim=pass (1024-bit key; unprotected) header.d=mendozajonas.com header.i=@mendozajonas.com header.b=IA8JZO2z; dkim-atps=neutral Received: from skellige.ozlabs.ibm.com (unknown [122.99.82.10]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: sam@mendozajonas.com) by mendozajonas.com (Postfix) with ESMTPSA id C716D143FA4; Tue, 6 Sep 2016 09:46:06 +0800 (SGT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=mendozajonas.com; s=mail; t=1473126367; bh=xqhUY0BpetFJfnwjkLszGwnLXj7AVSrblxDOhZLo3dg=; h=From:To:Cc:Subject:Date:In-Reply-To:References:From; b=IA8JZO2zv52+KTCaukqBhEo1LNkfTTUuomE2C4F9zz4YtewuXP6lKb0SIT6MT53L0 uJmDKATbwsPYN0yZ5Qlxllpwmxr9B+yCJSoYtSZEsXqFP9rgvJvPxbI24nv75DJMjL wyiDA6tAm4BYEGC91JY5Nbe9cxI3EQxgUtieRswc= From: Samuel Mendoza-Jonas To: petitboot@lists.ozlabs.org Subject: [PATCH 3/4] discover: Pass UUID to discover_device_create() Date: Tue, 6 Sep 2016 11:45:53 +1000 Message-Id: <20160906014554.20634-4-sam@mendozajonas.com> X-Mailer: git-send-email 2.9.3 In-Reply-To: <20160906014554.20634-1-sam@mendozajonas.com> References: <20160906014554.20634-1-sam@mendozajonas.com> X-BeenThere: petitboot@lists.ozlabs.org X-Mailman-Version: 2.1.22 Precedence: list List-Id: Petitboot bootloader development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Samuel Mendoza-Jonas MIME-Version: 1.0 Errors-To: petitboot-bounces+incoming=patchwork.ozlabs.org@lists.ozlabs.org Sender: "Petitboot" Currently discover_device_create() will search for existing discover devices by id to determine if a new device is required. However it is possible under some circumstances for distinct devices to have the same name. This is especially troublesome if the following network events are seen in network_handle_nlmsg(): - New interface, 'foo' with uuid x:x:x:x:x:x -> new discover device created with dev->device->id = 'foo' dev->uuid = x:x:x:x:x:x - New interface, 'foo' with uuid y:y:y:y:y:y -> existing device 'foo' found dev->uuid = y:y:y:y:y:y This can occur if an interface rename event arrives *after* an old name is reused, where temporarily Petitboot will see two distinct network interfaces with the same name. Now the two interfaces point to the same discover device, which can quickly result in a segfault if a 'remove' event occurs for one of the interfaces and the discover device is freed. To generally avoid this a 'uuid' parameter is added to discover_device_create(), which if present allows existing devices to be looked up by UUID rather than just their name. Signed-off-by: Samuel Mendoza-Jonas --- discover/device-handler.c | 13 +++++++++---- discover/device-handler.h | 2 +- discover/network.c | 10 ++++++---- discover/udev.c | 2 +- discover/user-event.c | 15 ++++++++++++--- test/parser/utils.c | 2 +- 6 files changed, 30 insertions(+), 14 deletions(-) diff --git a/discover/device-handler.c b/discover/device-handler.c index 54a1986..70e4506 100644 --- a/discover/device-handler.c +++ b/discover/device-handler.c @@ -221,17 +221,22 @@ static int destroy_device(void *arg) } struct discover_device *discover_device_create(struct device_handler *handler, - const char *id) + const char *uuid, const char *id) { struct discover_device *dev; - dev = device_lookup_by_id(handler, id); + if (uuid) + dev = device_lookup_by_uuid(handler, uuid); + else + dev = device_lookup_by_id(handler, id); + if (dev) return dev; dev = talloc_zero(handler, struct discover_device); dev->device = talloc_zero(dev, struct device); dev->device->id = talloc_strdup(dev->device, id); + dev->uuid = talloc_strdup(dev, uuid); list_init(&dev->params); list_init(&dev->boot_options); @@ -1138,7 +1143,7 @@ void device_handler_process_url(struct device_handler *handler, goto msg; } - dev = discover_device_create(handler, event->device); + dev = discover_device_create(handler, mac, event->device); if (pb_url->scheme == pb_url_file) dev->device->type = DEVICE_TYPE_ANY; ctx = device_handler_discover_context_create(handler, dev); @@ -1171,7 +1176,7 @@ void device_handler_discover_context_commit(struct device_handler *handler, struct discover_device *dev = ctx->device; struct discover_boot_option *opt, *tmp; - if (!device_lookup_by_id(handler, dev->device->id)) + if (!device_lookup_by_uuid(handler, dev->uuid)) device_handler_add_device(handler, dev); /* move boot options from the context to the device */ diff --git a/discover/device-handler.h b/discover/device-handler.h index 6ffa4e1..d2e3baf 100644 --- a/discover/device-handler.h +++ b/discover/device-handler.h @@ -81,7 +81,7 @@ const struct discover_device *device_handler_get_device( const struct device_handler *handler, unsigned int index); struct discover_device *discover_device_create(struct device_handler *handler, - const char *id); + const char *uuid, const char *id); void device_handler_add_device(struct device_handler *handler, struct discover_device *device); void device_handler_add_ramdisk(struct device_handler *handler, diff --git a/discover/network.c b/discover/network.c index 2de96ec..0161c69 100644 --- a/discover/network.c +++ b/discover/network.c @@ -197,13 +197,15 @@ static char *mac_bytes_to_string(void *ctx, uint8_t *addr, int len) static void add_interface(struct network *network, struct interface *interface) { + char *uuid = mac_bytes_to_string(interface, interface->hwaddr, + sizeof(interface->hwaddr)); + list_add(&network->interfaces, &interface->list); - interface->dev = discover_device_create(network->handler, - interface->name); + interface->dev = discover_device_create(network->handler, uuid, + interface->name); interface->dev->device->type = DEVICE_TYPE_NETWORK; - interface->dev->uuid = mac_bytes_to_string(interface->dev, - interface->hwaddr, sizeof(interface->hwaddr)); device_handler_add_device(network->handler, interface->dev); + talloc_free(uuid); } static void remove_interface(struct network *network, diff --git a/discover/udev.c b/discover/udev.c index f4cefab..1e04313 100644 --- a/discover/udev.c +++ b/discover/udev.c @@ -158,7 +158,7 @@ static int udev_handle_block_add(struct pb_udev *udev, struct udev_device *dev, } } - ddev = discover_device_create(udev->handler, name); + ddev = discover_device_create(udev->handler, uuid, name); ddev->device_path = talloc_strdup(ddev, node); diff --git a/discover/user-event.c b/discover/user-event.c index 7ceddb1..20b2bea 100644 --- a/discover/user-event.c +++ b/discover/user-event.c @@ -385,7 +385,8 @@ static int user_event_dhcp(struct user_event *uev, struct event *event) struct device_handler *handler = uev->handler; struct discover_device *dev; - dev = discover_device_create(handler, event->device); + dev = discover_device_create(handler, event_get_param(event, "mac"), + event->device); device_handler_dhcp(handler, dev, event); @@ -398,7 +399,10 @@ static int user_event_add(struct user_event *uev, struct event *event) struct discover_context *ctx; struct discover_device *dev; - dev = discover_device_create(handler, event->device); + /* In case this is a network interface, try to refer to it by UUID */ + dev = discover_device_create(handler, event_get_param(event, "mac"), + event->device); + dev->device->id = talloc_strdup(dev, event->device); ctx = device_handler_discover_context_create(handler, dev); parse_user_event(ctx, event); @@ -414,8 +418,13 @@ static int user_event_remove(struct user_event *uev, struct event *event) { struct device_handler *handler = uev->handler; struct discover_device *dev; + const char *mac = event_get_param(event, "mac"); + + if (mac) + dev = device_lookup_by_uuid(handler, event_get_param(event, "mac")); + else + dev = device_lookup_by_id(handler, event->device); - dev = device_lookup_by_id(handler, event->device); if (!dev) return 0; diff --git a/test/parser/utils.c b/test/parser/utils.c index 5cebc99..f0796fd 100644 --- a/test/parser/utils.c +++ b/test/parser/utils.c @@ -73,7 +73,7 @@ struct discover_device *test_create_device(struct parser_test *test, { struct discover_device *dev; - dev = discover_device_create(test->handler, name); + dev = discover_device_create(test->handler, NULL, name); dev->device->id = talloc_strdup(dev, name); dev->device_path = talloc_asprintf(dev, "/dev/%s", name);