Let's protect all vmcore modifications by the vmcore_mutex and
disallow vmcore modifications after the vmcore was opened: modifications
would no longer be safe. Properly synchronize against concurrent opening
of the vmcore.

As a nice side-effect, we now properly protect concurrent vmcore
modifications.

No need to grab the mutex during mmap()/read(): after we opened the
vmcore, modifications are impossible.

Signed-off-by: David Hildenbrand <da...@redhat.com>
---
 fs/proc/vmcore.c | 42 +++++++++++++++++++-----------------------
 1 file changed, 19 insertions(+), 23 deletions(-)

diff --git a/fs/proc/vmcore.c b/fs/proc/vmcore.c
index b91c304463c9..6371dbaa21be 100644
--- a/fs/proc/vmcore.c
+++ b/fs/proc/vmcore.c
@@ -243,33 +243,27 @@ static int vmcoredd_copy_dumps(struct iov_iter *iter, u64 
start, size_t size)
 {
        struct vmcoredd_node *dump;
        u64 offset = 0;
-       int ret = 0;
        size_t tsz;
        char *buf;
 
-       mutex_lock(&vmcore_mutex);
        list_for_each_entry(dump, &vmcoredd_list, list) {
                if (start < offset + dump->size) {
                        tsz = min(offset + (u64)dump->size - start, (u64)size);
                        buf = dump->buf + start - offset;
-                       if (copy_to_iter(buf, tsz, iter) < tsz) {
-                               ret = -EFAULT;
-                               goto out_unlock;
-                       }
+                       if (copy_to_iter(buf, tsz, iter) < tsz)
+                               return -EFAULT;
 
                        size -= tsz;
                        start += tsz;
 
                        /* Leave now if buffer filled already */
                        if (!size)
-                               goto out_unlock;
+                               return 0;
                }
                offset += dump->size;
        }
 
-out_unlock:
-       mutex_unlock(&vmcore_mutex);
-       return ret;
+       return 0;
 }
 
 #ifdef CONFIG_MMU
@@ -278,20 +272,16 @@ static int vmcoredd_mmap_dumps(struct vm_area_struct 
*vma, unsigned long dst,
 {
        struct vmcoredd_node *dump;
        u64 offset = 0;
-       int ret = 0;
        size_t tsz;
        char *buf;
 
-       mutex_lock(&vmcore_mutex);
        list_for_each_entry(dump, &vmcoredd_list, list) {
                if (start < offset + dump->size) {
                        tsz = min(offset + (u64)dump->size - start, (u64)size);
                        buf = dump->buf + start - offset;
                        if (remap_vmalloc_range_partial(vma, dst, buf, 0,
-                                                       tsz)) {
-                               ret = -EFAULT;
-                               goto out_unlock;
-                       }
+                                                       tsz))
+                               return -EFAULT;
 
                        size -= tsz;
                        start += tsz;
@@ -299,14 +289,12 @@ static int vmcoredd_mmap_dumps(struct vm_area_struct 
*vma, unsigned long dst,
 
                        /* Leave now if buffer filled already */
                        if (!size)
-                               goto out_unlock;
+                               return 0;
                }
                offset += dump->size;
        }
 
-out_unlock:
-       mutex_unlock(&vmcore_mutex);
-       return ret;
+       return 0;
 }
 #endif /* CONFIG_MMU */
 #endif /* CONFIG_PROC_VMCORE_DEVICE_DUMP */
@@ -1482,6 +1470,10 @@ int vmcore_add_device_dump(struct vmcoredd_data *data)
                return -EINVAL;
        }
 
+       /* We'll recheck under lock later. */
+       if (data_race(vmcore_opened))
+               return -EBUSY;
+
        if (!data || !strlen(data->dump_name) ||
            !data->vmcoredd_callback || !data->size)
                return -EINVAL;
@@ -1515,12 +1507,16 @@ int vmcore_add_device_dump(struct vmcoredd_data *data)
        dump->buf = buf;
        dump->size = data_size;
 
-       /* Add the dump to driver sysfs list */
+       /* Add the dump to driver sysfs list and update the elfcore hdr */
        mutex_lock(&vmcore_mutex);
-       list_add_tail(&dump->list, &vmcoredd_list);
-       mutex_unlock(&vmcore_mutex);
+       if (vmcore_opened) {
+               ret = -EBUSY;
+               goto out_err;
+       }
 
+       list_add_tail(&dump->list, &vmcoredd_list);
        vmcoredd_update_size(data_size);
+       mutex_unlock(&vmcore_mutex);
        return 0;
 
 out_err:
-- 
2.46.1


Reply via email to