Skip to contentSkip to conversation

input: Fix crash when SDP record isn't available

PATCH In review Bastien Nocera 2 participants 4 messages Dec 7, 2013 – Dec 10, 2013 Bluetooth drivers

lore
Share & tools
lore
#0 lore raw

PATCH 2/2 input: Fix crash when SDP record isn't available

On startup, if the SDP cache has been removed but the pairing information is still present, we'd crash trying to access inside a NULL record struct.

---
 profiles/input/device.c | 3 +++
 1 file changed, 3 insertions(+)
profiles/input/device.c +3 -0
index 521aca8..62f6dbb 100644
@@ -811,6 +811,9 @@ static struct input_device *input_device_new(struct btd_service *service)
 	struct input_device *idev;
 	char name[HCI_MAX_NAME_LENGTH + 1];
 
+	if (!rec)
+		return NULL;
+
 	idev = g_new0(struct input_device, 1);
 	bacpy(&idev->src, btd_adapter_get_address(adapter));
 	bacpy(&idev->dst, device_get_address(device));
Signature · 1 line
1.8.4.2
#1 lore raw

Hi Bastien,

Quoted 12 lines · Bastien Nocera

On startup, if the SDP cache has been removed but the pairing information is still present, we'd crash trying to access inside a NULL record struct.

---
 profiles/input/device.c | 3 +++
 1 file changed, 3 insertions(+)
profiles/input/device.c +0 -0
index 521aca8..62f6dbb 100644

profiles/input/device.c static struct input_device *input_device_new(struct btd_service *service) · line 819

Quoted 9 lines
profiles/input/device.c +3 -0
@@ -811,6 +811,9 @@ static struct input_device *input_device_new(struct btd_service *service)
 	struct input_device *idev;
 	char name[HCI_MAX_NAME_LENGTH + 1];
 
+	if (!rec)
+		return NULL;
+
 	idev = g_new0(struct input_device, 1);
 	bacpy(&idev->src, btd_adapter_get_address(adapter));
 	bacpy(&idev->dst, device_get_address(device));

I've applied your first patch, but I'd like to understand a bit better how you'd end up in this situation. Is this accomplishable through BlueZ APIs or only if one goes and messes with the storage directly? Either way, is this the best approach or should we consider still creating the input device but just ignore the bits that depend on the SDP record?

Johan

#2 lore raw
Quoted 18 lines · Johan Hedberg

Hi Bastien,

Quoted 15 lines · Bastien Nocera

On startup, if the SDP cache has been removed but the pairing information is still present, we'd crash trying to access inside a NULL record struct.

---
 profiles/input/device.c | 3 +++
 1 file changed, 3 insertions(+)
profiles/input/device.c +0 -0
index 521aca8..62f6dbb 100644
@@ -811,6 +811,9 @@ static struct input_device *input_device_new(struct btd_service *service)
 	struct input_device *idev;
 	char name[HCI_MAX_NAME_LENGTH + 1];
 
Quoted 12 lines

profiles/input/device.c static struct input_device *input_device_new(struct btd_service *service) · line 819

Quoted 6 lines
profiles/input/device.c +3 -0
+	if (!rec)
+		return NULL;
+
 	idev = g_new0(struct input_device, 1);
 	bacpy(&idev->src, btd_adapter_get_address(adapter));
 	bacpy(&idev->dst, device_get_address(device));

I've applied your first patch, but I'd like to understand a bit better how you'd end up in this situation. Is this accomplishable through BlueZ APIs or only if one goes and messes with the storage directly? Either way, is this the best approach or should we consider still creating the input device but just ignore the bits that depend on the SDP record?

I rm'ed the file in the cache/ directory. I'm not too fussed about what the end result actually is, it just shouldn't crash. The same patch with a warning that you shouldn't fiddle with the cache directory would be fine as well ;)

#3 lore raw

Hi Bastien,

Quoted 32 lines · Bastien Nocera
Quoted 18 lines · Johan Hedberg

Hi Bastien,

Quoted 15 lines · Bastien Nocera

On startup, if the SDP cache has been removed but the pairing information is still present, we'd crash trying to access inside a NULL record struct.

---
 profiles/input/device.c | 3 +++
 1 file changed, 3 insertions(+)
profiles/input/device.c +0 -0
index 521aca8..62f6dbb 100644
@@ -811,6 +811,9 @@ static struct input_device *input_device_new(struct btd_service *service)
 	struct input_device *idev;
 	char name[HCI_MAX_NAME_LENGTH + 1];
 
Quoted 12 lines

profiles/input/device.c static struct input_device *input_device_new(struct btd_service *service) · line 819

Quoted 6 lines
profiles/input/device.c +3 -0
+	if (!rec)
+		return NULL;
+
 	idev = g_new0(struct input_device, 1);
 	bacpy(&idev->src, btd_adapter_get_address(adapter));
 	bacpy(&idev->dst, device_get_address(device));

I've applied your first patch, but I'd like to understand a bit better how you'd end up in this situation. Is this accomplishable through BlueZ APIs or only if one goes and messes with the storage directly? Either way, is this the best approach or should we consider still creating the input device but just ignore the bits that depend on the SDP record?

Quoted 4 lines

I rm'ed the file in the cache/ directory. I'm not too fussed about what the end result actually is, it just shouldn't crash. The same patch with a warning that you shouldn't fiddle with the cache directory would be fine as well ;)

Fair enough. I've applied the patch now. Thanks.

Johan