From fb2c445277e7b0b4ffe10de8114bad4eccaca948 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Mon, 13 Nov 2017 10:06:08 +0100 Subject: [PATCH 1/7] ANDROID: binder: fix transaction leak. If a call to put_user() fails, we failed to properly free a transaction and send a failed reply (if necessary). Signed-off-by: Martijn Coenen Cc: stable # 4.14 Signed-off-by: Greg Kroah-Hartman --- drivers/android/binder.c | 40 +++++++++++++++++++++++++++++++--------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index a73596a4f804..bccec9de0533 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -1947,6 +1947,26 @@ static void binder_send_failed_reply(struct binder_transaction *t, } } +/** + * binder_cleanup_transaction() - cleans up undelivered transaction + * @t: transaction that needs to be cleaned up + * @reason: reason the transaction wasn't delivered + * @error_code: error to return to caller (if synchronous call) + */ +static void binder_cleanup_transaction(struct binder_transaction *t, + const char *reason, + uint32_t error_code) +{ + if (t->buffer->target_node && !(t->flags & TF_ONE_WAY)) { + binder_send_failed_reply(t, error_code); + } else { + binder_debug(BINDER_DEBUG_DEAD_TRANSACTION, + "undelivered transaction %d, %s\n", + t->debug_id, reason); + binder_free_transaction(t); + } +} + /** * binder_validate_object() - checks for a valid metadata object in a buffer. * @buffer: binder_buffer that we're parsing. @@ -4015,12 +4035,20 @@ retry: if (put_user(cmd, (uint32_t __user *)ptr)) { if (t_from) binder_thread_dec_tmpref(t_from); + + binder_cleanup_transaction(t, "put_user failed", + BR_FAILED_REPLY); + return -EFAULT; } ptr += sizeof(uint32_t); if (copy_to_user(ptr, &tr, sizeof(tr))) { if (t_from) binder_thread_dec_tmpref(t_from); + + binder_cleanup_transaction(t, "copy_to_user failed", + BR_FAILED_REPLY); + return -EFAULT; } ptr += sizeof(tr); @@ -4090,15 +4118,9 @@ static void binder_release_work(struct binder_proc *proc, struct binder_transaction *t; t = container_of(w, struct binder_transaction, work); - if (t->buffer->target_node && - !(t->flags & TF_ONE_WAY)) { - binder_send_failed_reply(t, BR_DEAD_REPLY); - } else { - binder_debug(BINDER_DEBUG_DEAD_TRANSACTION, - "undelivered transaction %d\n", - t->debug_id); - binder_free_transaction(t); - } + + binder_cleanup_transaction(t, "process died.", + BR_DEAD_REPLY); } break; case BINDER_WORK_RETURN_ERROR: { struct binder_error *e = container_of( From 7fa32e5ec28b1609abc0b797b58267f725fc3964 Mon Sep 17 00:00:00 2001 From: "K. Y. Srinivasan" Date: Tue, 14 Nov 2017 06:53:33 -0700 Subject: [PATCH 2/7] Drivers: hv: vmbus: Fix a rescind issue The current rescind processing code will not correctly handle the case where the host immediately rescinds a channel that has been offerred. In this case, we could be blocked in the open call and since the channel is rescinded, the host will not respond and we could be blocked forever in the vmbus open call.i Fix this problem. Signed-off-by: K. Y. Srinivasan Cc: stable@vger.kernel.org Signed-off-by: Greg Kroah-Hartman --- drivers/hv/channel.c | 10 ++++++++-- drivers/hv/channel_mgmt.c | 7 ++++--- include/linux/hyperv.h | 1 + 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/hv/channel.c b/drivers/hv/channel.c index 19f0cf37e0ed..ba0a092ae085 100644 --- a/drivers/hv/channel.c +++ b/drivers/hv/channel.c @@ -659,22 +659,28 @@ void vmbus_close(struct vmbus_channel *channel) */ return; } - mutex_lock(&vmbus_connection.channel_mutex); /* * Close all the sub-channels first and then close the * primary channel. */ list_for_each_safe(cur, tmp, &channel->sc_list) { cur_channel = list_entry(cur, struct vmbus_channel, sc_list); - vmbus_close_internal(cur_channel); if (cur_channel->rescind) { + wait_for_completion(&cur_channel->rescind_event); + mutex_lock(&vmbus_connection.channel_mutex); + vmbus_close_internal(cur_channel); hv_process_channel_removal( cur_channel->offermsg.child_relid); + } else { + mutex_lock(&vmbus_connection.channel_mutex); + vmbus_close_internal(cur_channel); } + mutex_unlock(&vmbus_connection.channel_mutex); } /* * Now close the primary. */ + mutex_lock(&vmbus_connection.channel_mutex); vmbus_close_internal(channel); mutex_unlock(&vmbus_connection.channel_mutex); } diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c index ec5454f3f4a6..c21020b69114 100644 --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -333,6 +333,7 @@ static struct vmbus_channel *alloc_channel(void) return NULL; spin_lock_init(&channel->lock); + init_completion(&channel->rescind_event); INIT_LIST_HEAD(&channel->sc_list); INIT_LIST_HEAD(&channel->percpu_list); @@ -898,6 +899,7 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr) /* * Now wait for offer handling to complete. */ + vmbus_rescind_cleanup(channel); while (READ_ONCE(channel->probe_done) == false) { /* * We wait here until any channel offer is currently @@ -913,7 +915,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr) if (channel->device_obj) { if (channel->chn_rescind_callback) { channel->chn_rescind_callback(channel); - vmbus_rescind_cleanup(channel); return; } /* @@ -922,7 +923,6 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr) */ dev = get_device(&channel->device_obj->device); if (dev) { - vmbus_rescind_cleanup(channel); vmbus_device_unregister(channel->device_obj); put_device(dev); } @@ -936,13 +936,14 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr) * 2. Then close the primary channel. */ mutex_lock(&vmbus_connection.channel_mutex); - vmbus_rescind_cleanup(channel); if (channel->state == CHANNEL_OPEN_STATE) { /* * The channel is currently not open; * it is safe for us to cleanup the channel. */ hv_process_channel_removal(rescind->child_relid); + } else { + complete(&channel->rescind_event); } mutex_unlock(&vmbus_connection.channel_mutex); } diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h index f3e97c5f94c9..6c9336626592 100644 --- a/include/linux/hyperv.h +++ b/include/linux/hyperv.h @@ -708,6 +708,7 @@ struct vmbus_channel { u8 monitor_bit; bool rescind; /* got rescind msg */ + struct completion rescind_event; u32 ringbuffer_gpadlhandle; From 297d6b6e56c2977fc504c61bbeeaa21296923f89 Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Tue, 14 Nov 2017 13:06:47 -0700 Subject: [PATCH 3/7] hv: kvp: Avoid reading past allocated blocks from KVP file While reading in more than one block (50) of KVP records, the allocation goes per block, but the reads used the total number of allocated records (without resetting the pointer/stream). This causes the records buffer to overrun when the refresh reads more than one block over the previous capacity (e.g. reading more than 100 KVP records whereas the in-memory database was empty before). Fix this by reading the correct number of KVP records from file each time. Signed-off-by: Paul Meyer Signed-off-by: Long Li Cc: stable@vger.kernel.org Signed-off-by: K. Y. Srinivasan Signed-off-by: Greg Kroah-Hartman --- tools/hv/hv_kvp_daemon.c | 70 ++++++++-------------------------------- 1 file changed, 14 insertions(+), 56 deletions(-) diff --git a/tools/hv/hv_kvp_daemon.c b/tools/hv/hv_kvp_daemon.c index eaa3bec273c8..4c99c57736ce 100644 --- a/tools/hv/hv_kvp_daemon.c +++ b/tools/hv/hv_kvp_daemon.c @@ -193,11 +193,14 @@ static void kvp_update_mem_state(int pool) for (;;) { readp = &record[records_read]; records_read += fread(readp, sizeof(struct kvp_record), - ENTRIES_PER_BLOCK * num_blocks, - filep); + ENTRIES_PER_BLOCK * num_blocks - records_read, + filep); if (ferror(filep)) { - syslog(LOG_ERR, "Failed to read file, pool: %d", pool); + syslog(LOG_ERR, + "Failed to read file, pool: %d; error: %d %s", + pool, errno, strerror(errno)); + kvp_release_lock(pool); exit(EXIT_FAILURE); } @@ -210,6 +213,7 @@ static void kvp_update_mem_state(int pool) if (record == NULL) { syslog(LOG_ERR, "malloc failed"); + kvp_release_lock(pool); exit(EXIT_FAILURE); } continue; @@ -224,15 +228,11 @@ static void kvp_update_mem_state(int pool) fclose(filep); kvp_release_lock(pool); } + static int kvp_file_init(void) { int fd; - FILE *filep; - size_t records_read; char *fname; - struct kvp_record *record; - struct kvp_record *readp; - int num_blocks; int i; int alloc_unit = sizeof(struct kvp_record) * ENTRIES_PER_BLOCK; @@ -246,61 +246,19 @@ static int kvp_file_init(void) for (i = 0; i < KVP_POOL_COUNT; i++) { fname = kvp_file_info[i].fname; - records_read = 0; - num_blocks = 1; sprintf(fname, "%s/.kvp_pool_%d", KVP_CONFIG_LOC, i); fd = open(fname, O_RDWR | O_CREAT | O_CLOEXEC, 0644 /* rw-r--r-- */); if (fd == -1) return 1; - - filep = fopen(fname, "re"); - if (!filep) { - close(fd); - return 1; - } - - record = malloc(alloc_unit * num_blocks); - if (record == NULL) { - fclose(filep); - close(fd); - return 1; - } - for (;;) { - readp = &record[records_read]; - records_read += fread(readp, sizeof(struct kvp_record), - ENTRIES_PER_BLOCK, - filep); - - if (ferror(filep)) { - syslog(LOG_ERR, "Failed to read file, pool: %d", - i); - exit(EXIT_FAILURE); - } - - if (!feof(filep)) { - /* - * We have more data to read. - */ - num_blocks++; - record = realloc(record, alloc_unit * - num_blocks); - if (record == NULL) { - fclose(filep); - close(fd); - return 1; - } - continue; - } - break; - } kvp_file_info[i].fd = fd; - kvp_file_info[i].num_blocks = num_blocks; - kvp_file_info[i].records = record; - kvp_file_info[i].num_records = records_read; - fclose(filep); - + kvp_file_info[i].num_blocks = 1; + kvp_file_info[i].records = malloc(alloc_unit); + if (kvp_file_info[i].records == NULL) + return 1; + kvp_file_info[i].num_records = 0; + kvp_update_mem_state(i); } return 0; From 811d7e0215fb738fb9a9f0bcb1276516ad161ed1 Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Wed, 15 Nov 2017 13:00:43 -0800 Subject: [PATCH 4/7] firmware: vpd: Destroy vpd sections in remove function vpd sections are initialized during probe and thus should be destroyed in the remove function. Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver") Cc: stable Signed-off-by: Guenter Roeck Reviewed-by: Dmitry Torokhov Tested-by: Randy Dunlap Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/google/vpd.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c index 35e553b3b190..84217172297b 100644 --- a/drivers/firmware/google/vpd.c +++ b/drivers/firmware/google/vpd.c @@ -298,8 +298,17 @@ static int vpd_probe(struct platform_device *pdev) return vpd_sections_init(entry.cbmem_addr); } +static int vpd_remove(struct platform_device *pdev) +{ + vpd_section_destroy(&ro_vpd); + vpd_section_destroy(&rw_vpd); + + return 0; +} + static struct platform_driver vpd_driver = { .probe = vpd_probe, + .remove = vpd_remove, .driver = { .name = "vpd", }, @@ -324,8 +333,6 @@ static int __init vpd_platform_init(void) static void __exit vpd_platform_exit(void) { - vpd_section_destroy(&ro_vpd); - vpd_section_destroy(&rw_vpd); kobject_put(vpd_kobj); } From e4b28b3c3a405b251fa25db58abe1512814a680a Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Wed, 15 Nov 2017 13:00:44 -0800 Subject: [PATCH 5/7] firmware: vpd: Tie firmware kobject to device lifetime It doesn't make sense to have /sys/firmware/vpd if the device is not instantiated, so tie its lifetime to the device. Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver") Signed-off-by: Guenter Roeck Cc: stable Reviewed-by: Dmitry Torokhov Tested-by: Randy Dunlap Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/google/vpd.c | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c index 84217172297b..942e358efa60 100644 --- a/drivers/firmware/google/vpd.c +++ b/drivers/firmware/google/vpd.c @@ -295,7 +295,17 @@ static int vpd_probe(struct platform_device *pdev) if (ret) return ret; - return vpd_sections_init(entry.cbmem_addr); + vpd_kobj = kobject_create_and_add("vpd", firmware_kobj); + if (!vpd_kobj) + return -ENOMEM; + + ret = vpd_sections_init(entry.cbmem_addr); + if (ret) { + kobject_put(vpd_kobj); + return ret; + } + + return 0; } static int vpd_remove(struct platform_device *pdev) @@ -303,6 +313,8 @@ static int vpd_remove(struct platform_device *pdev) vpd_section_destroy(&ro_vpd); vpd_section_destroy(&rw_vpd); + kobject_put(vpd_kobj); + return 0; } @@ -322,10 +334,6 @@ static int __init vpd_platform_init(void) if (IS_ERR(pdev)) return PTR_ERR(pdev); - vpd_kobj = kobject_create_and_add("vpd", firmware_kobj); - if (!vpd_kobj) - return -ENOMEM; - platform_driver_register(&vpd_driver); return 0; @@ -333,7 +341,6 @@ static int __init vpd_platform_init(void) static void __exit vpd_platform_exit(void) { - kobject_put(vpd_kobj); } module_init(vpd_platform_init); From 0631fb8b027f5968c2f5031f0b3ff7be3e4bebcc Mon Sep 17 00:00:00 2001 From: Guenter Roeck Date: Wed, 15 Nov 2017 13:00:45 -0800 Subject: [PATCH 6/7] firmware: vpd: Fix platform driver and device registration/unregistration The driver exit function needs to unregister both platform device and driver. Also, during registration, register driver first and perform error checks. Fixes: 049a59db34eb ("firmware: Google VPD sysfs driver") Signed-off-by: Guenter Roeck Cc: stable Tested-by: Randy Dunlap Reviewed-by: Dmitry Torokhov Signed-off-by: Greg Kroah-Hartman --- drivers/firmware/google/vpd.c | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/drivers/firmware/google/vpd.c b/drivers/firmware/google/vpd.c index 942e358efa60..e4b40f2b4627 100644 --- a/drivers/firmware/google/vpd.c +++ b/drivers/firmware/google/vpd.c @@ -326,21 +326,29 @@ static struct platform_driver vpd_driver = { }, }; +static struct platform_device *vpd_pdev; + static int __init vpd_platform_init(void) { - struct platform_device *pdev; + int ret; - pdev = platform_device_register_simple("vpd", -1, NULL, 0); - if (IS_ERR(pdev)) - return PTR_ERR(pdev); + ret = platform_driver_register(&vpd_driver); + if (ret) + return ret; - platform_driver_register(&vpd_driver); + vpd_pdev = platform_device_register_simple("vpd", -1, NULL, 0); + if (IS_ERR(vpd_pdev)) { + platform_driver_unregister(&vpd_driver); + return PTR_ERR(vpd_pdev); + } return 0; } static void __exit vpd_platform_exit(void) { + platform_device_unregister(vpd_pdev); + platform_driver_unregister(&vpd_driver); } module_init(vpd_platform_init); From 66bc5df31110652a31c91f14b4e23f7c51e5328e Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Tue, 5 Dec 2017 12:40:17 +0100 Subject: [PATCH 7/7] MAINTAINERS: update Android driver maintainers. Add Todd Kjos and myself, remove Riley (who no longer works at Google). Signed-off-by: Martijn Coenen Signed-off-by: Greg Kroah-Hartman --- MAINTAINERS | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/MAINTAINERS b/MAINTAINERS index aa71ab52fd76..da8264fc09d4 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -859,7 +859,8 @@ F: kernel/configs/android* ANDROID DRIVERS M: Greg Kroah-Hartman M: Arve Hjønnevåg -M: Riley Andrews +M: Todd Kjos +M: Martijn Coenen T: git git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/staging.git L: devel@driverdev.osuosl.org S: Supported