September 2018 - Linux-stable-mirror

[PATCH AUTOSEL 4.18 001/113] cifs: check if SMB2 PDU size has been padded and suppress the warning

by Sasha Levin

From: Ronnie Sahlberg <lsahlber(a)redhat.com> [ Upstream commit e6c47dd0da1e3a484e778046fc10da0b20606a86 ] Some SMB2/3 servers, Win2016 but possibly others too, adds padding not only between PDUs in a compound but also to the final PDU. This padding extends the PDU to a multiple of 8 bytes. Check if the unexpected length looks like this might be the case and avoid triggering the log messages for : "SMB2 server sent bad RFC1001 len %d not %d\n" Signed-off-by: Ronnie Sahlberg <lsahlber(a)redhat.com> Signed-off-by: Steve French <stfrench(a)microsoft.com> Signed-off-by: Sasha Levin <alexander.levin(a)microsoft.com> --- fs/cifs/smb2misc.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/fs/cifs/smb2misc.c b/fs/cifs/smb2misc.c index 3ff7cec2da81..239215dcc00b 100644 --- a/fs/cifs/smb2misc.c +++ b/fs/cifs/smb2misc.c @@ -240,6 +240,13 @@ smb2_check_message(char *buf, unsigned int len, struct TCP_Server_Info *srvr) if (clc_len == len + 1) return 0; + /* + * Some windows servers (win2016) will pad also the final + * PDU in a compound to 8 bytes. + */ + if (((clc_len + 7) & ~7) == len) + return 0; + /* * MacOS server pads after SMB2.1 write response with 3 bytes * of junk. Other servers match RFC1001 len to actual -- 2.17.1

5 years, 7 months

4
71
0 0

[PATCH 2/7] mm/rmap: map_pte() was not handling private ZONE_DEVICE page properly

by jglisse＠redhat.com

From: Ralph Campbell <rcampbell(a)nvidia.com> Private ZONE_DEVICE pages use a special pte entry and thus are not present. Properly handle this case in map_pte(), it is already handled in check_pte(), the map_pte() part was lost in some rebase most probably. Without this patch the slow migration path can not migrate back private ZONE_DEVICE memory to regular memory. This was found after stress testing migration back to system memory. This ultimatly can lead the CPU to an infinite page fault loop on the special swap entry. Signed-off-by: Ralph Campbell <rcampbell(a)nvidia.com> Signed-off-by: Jérôme Glisse <jglisse(a)redhat.com> Cc: Andrew Morton <akpm(a)linux-foundation.org> Cc: Kirill A. Shutemov <kirill.shutemov(a)linux.intel.com> Cc: stable(a)vger.kernel.org --- mm/page_vma_mapped.c | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/mm/page_vma_mapped.c b/mm/page_vma_mapped.c index ae3c2a35d61b..1cf5b9bfb559 100644 --- a/mm/page_vma_mapped.c +++ b/mm/page_vma_mapped.c @@ -21,6 +21,15 @@ static bool map_pte(struct page_vma_mapped_walk *pvmw) if (!is_swap_pte(*pvmw->pte)) return false; } else { + if (is_swap_pte(*pvmw->pte)) { + swp_entry_t entry; + + /* Handle un-addressable ZONE_DEVICE memory */ + entry = pte_to_swp_entry(*pvmw->pte); + if (is_device_private_entry(entry)) + return true; + } + if (!pte_present(*pvmw->pte)) return false; } -- 2.17.1

5 years, 7 months

3
6
0 0

[PATCH] staging: android: ion: fix ION_IOC_{MAP, SHARE} use-after-free

by Greg Hackmann

The ION_IOC_{MAP,SHARE} ioctls drop and reacquire client->lock several times while operating on one of the client's ion_handles. This creates windows where userspace can call ION_IOC_FREE on the same client with the same handle, and effectively make the kernel drop its own reference. For example: - thread A: ION_IOC_ALLOC creates an ion_handle with refcount 1 - thread A: starts ION_IOC_MAP and increments the refcount to 2 - thread B: ION_IOC_FREE decrements the refcount to 1 - thread B: ION_IOC_FREE decrements the refcount to 0 and frees the handle - thread A: continues ION_IOC_MAP with a dangling ion_handle * to freed memory Fix this by holding client->lock for the duration of ION_IOC_{MAP,SHARE}, preventing the concurrent ION_IOC_FREE. Also remove ion_handle_get_by_id(), since there's literally no way to use it safely. This patch is applied on top of 4.9.y. Kernels 4.12 and later are unaffected, since all the underlying ion_handle infrastructure has been ripped out. Cc: stable(a)vger.kernel.org # v4.11- Signed-off-by: Greg Hackmann <ghackmann(a)google.com> --- drivers/staging/android/ion/ion-ioctl.c | 12 ++++--- drivers/staging/android/ion/ion.c | 48 +++++++++++++++---------- drivers/staging/android/ion/ion_priv.h | 6 ++-- 3 files changed, 40 insertions(+), 26 deletions(-) diff --git a/drivers/staging/android/ion/ion-ioctl.c b/drivers/staging/android/ion/ion-ioctl.c index 2b700e8455c6..e3596855a703 100644 --- a/drivers/staging/android/ion/ion-ioctl.c +++ b/drivers/staging/android/ion/ion-ioctl.c @@ -128,11 +128,15 @@ long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { struct ion_handle *handle; - handle = ion_handle_get_by_id(client, data.handle.handle); - if (IS_ERR(handle)) + mutex_lock(&client->lock); + handle = ion_handle_get_by_id_nolock(client, data.handle.handle); + if (IS_ERR(handle)) { + mutex_unlock(&client->lock); return PTR_ERR(handle); - data.fd.fd = ion_share_dma_buf_fd(client, handle); - ion_handle_put(handle); + } + data.fd.fd = ion_share_dma_buf_fd_nolock(client, handle); + ion_handle_put_nolock(handle); + mutex_unlock(&client->lock); if (data.fd.fd < 0) ret = data.fd.fd; break; diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c index 6f9974cb0e15..403df8bf4b48 100644 --- a/drivers/staging/android/ion/ion.c +++ b/drivers/staging/android/ion/ion.c @@ -352,18 +352,6 @@ struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client, return handle ? handle : ERR_PTR(-EINVAL); } -struct ion_handle *ion_handle_get_by_id(struct ion_client *client, - int id) -{ - struct ion_handle *handle; - - mutex_lock(&client->lock); - handle = ion_handle_get_by_id_nolock(client, id); - mutex_unlock(&client->lock); - - return handle; -} - static bool ion_handle_validate(struct ion_client *client, struct ion_handle *handle) { @@ -1029,24 +1017,28 @@ static struct dma_buf_ops dma_buf_ops = { .kunmap = ion_dma_buf_kunmap, }; -struct dma_buf *ion_share_dma_buf(struct ion_client *client, - struct ion_handle *handle) +static struct dma_buf *__ion_share_dma_buf(struct ion_client *client, + struct ion_handle *handle, + bool lock_client) { DEFINE_DMA_BUF_EXPORT_INFO(exp_info); struct ion_buffer *buffer; struct dma_buf *dmabuf; bool valid_handle; - mutex_lock(&client->lock); + if (lock_client) + mutex_lock(&client->lock); valid_handle = ion_handle_validate(client, handle); if (!valid_handle) { WARN(1, "%s: invalid handle passed to share.\n", __func__); - mutex_unlock(&client->lock); + if (lock_client) + mutex_unlock(&client->lock); return ERR_PTR(-EINVAL); } buffer = handle->buffer; ion_buffer_get(buffer); - mutex_unlock(&client->lock); + if (lock_client) + mutex_unlock(&client->lock); exp_info.ops = &dma_buf_ops; exp_info.size = buffer->size; @@ -1061,14 +1053,21 @@ struct dma_buf *ion_share_dma_buf(struct ion_client *client, return dmabuf; } + +struct dma_buf *ion_share_dma_buf(struct ion_client *client, + struct ion_handle *handle) +{ + return __ion_share_dma_buf(client, handle, true); +} EXPORT_SYMBOL(ion_share_dma_buf); -int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle) +static int __ion_share_dma_buf_fd(struct ion_client *client, + struct ion_handle *handle, bool lock_client) { struct dma_buf *dmabuf; int fd; - dmabuf = ion_share_dma_buf(client, handle); + dmabuf = __ion_share_dma_buf(client, handle, lock_client); if (IS_ERR(dmabuf)) return PTR_ERR(dmabuf); @@ -1078,8 +1077,19 @@ int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle) return fd; } + +int ion_share_dma_buf_fd(struct ion_client *client, struct ion_handle *handle) +{ + return __ion_share_dma_buf_fd(client, handle, true); +} EXPORT_SYMBOL(ion_share_dma_buf_fd); +int ion_share_dma_buf_fd_nolock(struct ion_client *client, + struct ion_handle *handle) +{ + return __ion_share_dma_buf_fd(client, handle, false); +} + struct ion_handle *ion_import_dma_buf(struct ion_client *client, struct dma_buf *dmabuf) { diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h index 3c3b3245275d..760e41885448 100644 --- a/drivers/staging/android/ion/ion_priv.h +++ b/drivers/staging/android/ion/ion_priv.h @@ -463,11 +463,11 @@ void ion_free_nolock(struct ion_client *client, struct ion_handle *handle); int ion_handle_put_nolock(struct ion_handle *handle); -struct ion_handle *ion_handle_get_by_id(struct ion_client *client, - int id); - int ion_handle_put(struct ion_handle *handle); int ion_query_heaps(struct ion_client *client, struct ion_heap_query *query); +int ion_share_dma_buf_fd_nolock(struct ion_client *client, + struct ion_handle *handle); + #endif /* _ION_PRIV_H */ -- 2.19.0.rc1.350.ge57e33dbd1-goog

5 years, 7 months

3
6
0 0

FAILED: patch "[PATCH] KVM: x86: SVM: Call x86_spec_ctrl_set_guest/host() with" failed to apply to 4.4-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ >From 024d83cadc6b2af027e473720f3c3da97496c318 Mon Sep 17 00:00:00 2001 From: Thomas Gleixner <tglx@xxxxxxxxxxxxx> Date: Sun, 12 Aug 2018 20:41:45 +0200 Subject: [PATCH] KVM: x86: SVM: Call x86_spec_ctrl_set_guest/host() with interrupts disabled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mikhail reported the following lockdep splat: WARNING: possible irq lock inversion dependency detected CPU 0/KVM/10284 just changed the state of lock: 000000000d538a88 (&st->lock){+...}, at: speculative_store_bypass_update+0x10b/0x170 but this lock was taken by another, HARDIRQ-safe lock in the past: (&(&sighand->siglock)->rlock){-.-.} and interrupts could create inverse lock ordering between them. Possible interrupt unsafe locking scenario: CPU0 CPU1 ---- ---- lock(&st->lock); local_irq_disable(); lock(&(&sighand->siglock)->rlock); lock(&st->lock); <Interrupt> lock(&(&sighand->siglock)->rlock); *** DEADLOCK *** The code path which connects those locks is: speculative_store_bypass_update() ssb_prctl_set() do_seccomp() do_syscall_64() In svm_vcpu_run() speculative_store_bypass_update() is called with interupts enabled via x86_virt_spec_ctrl_set_guest/host(). This is actually a false positive, because GIF=0 so interrupts are disabled even if IF=1; however, we can easily move the invocations of x86_virt_spec_ctrl_set_guest/host() into the interrupt disabled region to cure it, and it's a good idea to keep the GIF=0/IF=1 area as small and self-contained as possible. Fixes: 1f50ddb4f418 ("x86/speculation: Handle HT correctly on AMD") Reported-by: Mikhail Gavrilov <mikhail.v.gavrilov(a)gmail.com> Signed-off-by: Thomas Gleixner <tglx(a)linutronix.de> Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov(a)gmail.com> Cc: Joerg Roedel <joro(a)8bytes.org> Cc: Paolo Bonzini <pbonzini(a)redhat.com> Cc: Radim Krčmář <rkrcmar(a)redhat.com> Cc: Matthew Wilcox <willy(a)infradead.org> Cc: Borislav Petkov <bp(a)suse.de> Cc: Konrad Rzeszutek Wilk <konrad.wilk(a)oracle.com> Cc: Tom Lendacky <thomas.lendacky(a)amd.com> Cc: kvm(a)vger.kernel.org Cc: x86(a)kernel.org Cc: stable(a)vger.kernel.org Signed-off-by: Paolo Bonzini <pbonzini(a)redhat.com> diff --git a/arch/x86/kvm/svm.c b/arch/x86/kvm/svm.c index 73e27a98456f..6276140044d0 100644 --- a/arch/x86/kvm/svm.c +++ b/arch/x86/kvm/svm.c @@ -5586,8 +5586,6 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu) clgi(); - local_irq_enable(); - /* * If this vCPU has touched SPEC_CTRL, restore the guest's value if * it's non-zero. Since vmentry is serialising on affected CPUs, there @@ -5596,6 +5594,8 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu) */ x86_spec_ctrl_set_guest(svm->spec_ctrl, svm->virt_spec_ctrl); + local_irq_enable(); + asm volatile ( "push %%" _ASM_BP "; \n\t" "mov %c[rbx](%[svm]), %%" _ASM_BX " \n\t" @@ -5718,12 +5718,12 @@ static void svm_vcpu_run(struct kvm_vcpu *vcpu) if (unlikely(!msr_write_intercepted(vcpu, MSR_IA32_SPEC_CTRL))) svm->spec_ctrl = native_read_msr(MSR_IA32_SPEC_CTRL); - x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl); - reload_tss(vcpu); local_irq_disable(); + x86_spec_ctrl_restore_host(svm->spec_ctrl, svm->virt_spec_ctrl); + vcpu->arch.cr2 = svm->vmcb->save.cr2; vcpu->arch.regs[VCPU_REGS_RAX] = svm->vmcb->save.rax; vcpu->arch.regs[VCPU_REGS_RSP] = svm->vmcb->save.rsp;

5 years, 7 months

1
0
0 0

FAILED: patch "[PATCH] x86/vdso: Fix vDSO build if a retpoline is emitted" failed to apply to 4.4-stable tree

by gregkh＠linuxfoundation.org

The patch below does not apply to the 4.4-stable tree. If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <stable(a)vger.kernel.org>. thanks, greg k-h ------------------ original commit in Linus's tree ------------------ >From 2e549b2ee0e358bc758480e716b881f9cabedb6a Mon Sep 17 00:00:00 2001 From: Andy Lutomirski <luto(a)kernel.org> Date: Thu, 16 Aug 2018 12:41:15 -0700 Subject: [PATCH] x86/vdso: Fix vDSO build if a retpoline is emitted Currently, if the vDSO ends up containing an indirect branch or call, GCC will emit the "external thunk" style of retpoline, and it will fail to link. Fix it by building the vDSO with inline retpoline thunks. I haven't seen any reports of this triggering on an unpatched kernel. Fixes: commit 76b043848fd2 ("x86/retpoline: Add initial retpoline support") Signed-off-by: Andy Lutomirski <luto(a)kernel.org> Signed-off-by: Thomas Gleixner <tglx(a)linutronix.de> Acked-by: Matt Rickard <matt(a)softrans.com.au> Cc: Borislav Petkov <bp(a)alien8.de> Cc: Jason Vas Dias <jason.vas.dias(a)gmail.com> Cc: David Woodhouse <dwmw2(a)infradead.org> Cc: Peter Zijlstra <peterz(a)infradead.org> Cc: Andi Kleen <ak(a)linux.intel.com> Cc: stable(a)vger.kernel.org Link: https://lkml.kernel.org/r/c76538cd3afbe19c6246c2d1715bc6a60bd63985.15344483… diff --git a/Makefile b/Makefile index a0650bf79606..7bab2e90e4e1 100644 --- a/Makefile +++ b/Makefile @@ -507,9 +507,13 @@ KBUILD_AFLAGS += $(call cc-option, -no-integrated-as) endif RETPOLINE_CFLAGS_GCC := -mindirect-branch=thunk-extern -mindirect-branch-register +RETPOLINE_VDSO_CFLAGS_GCC := -mindirect-branch=thunk-inline -mindirect-branch-register RETPOLINE_CFLAGS_CLANG := -mretpoline-external-thunk +RETPOLINE_VDSO_CFLAGS_CLANG := -mretpoline RETPOLINE_CFLAGS := $(call cc-option,$(RETPOLINE_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_CFLAGS_CLANG))) +RETPOLINE_VDSO_CFLAGS := $(call cc-option,$(RETPOLINE_VDSO_CFLAGS_GCC),$(call cc-option,$(RETPOLINE_VDSO_CFLAGS_CLANG))) export RETPOLINE_CFLAGS +export RETPOLINE_VDSO_CFLAGS KBUILD_CFLAGS += $(call cc-option,-fno-PIE) KBUILD_AFLAGS += $(call cc-option,-fno-PIE) diff --git a/arch/x86/entry/vdso/Makefile b/arch/x86/entry/vdso/Makefile index 9f695f517747..fa3f439f0a92 100644 --- a/arch/x86/entry/vdso/Makefile +++ b/arch/x86/entry/vdso/Makefile @@ -68,9 +68,9 @@ $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE CFL := $(PROFILING) -mcmodel=small -fPIC -O2 -fasynchronous-unwind-tables -m64 \ $(filter -g%,$(KBUILD_CFLAGS)) $(call cc-option, -fno-stack-protector) \ -fno-omit-frame-pointer -foptimize-sibling-calls \ - -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO + -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO $(RETPOLINE_VDSO_CFLAGS) -$(vobjs): KBUILD_CFLAGS := $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS)) $(CFL) +$(vobjs): KBUILD_CFLAGS := $(filter-out $(GCC_PLUGINS_CFLAGS) $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS)) $(CFL) # # vDSO code runs in userspace and -pg doesn't help with profiling anyway. @@ -132,11 +132,13 @@ KBUILD_CFLAGS_32 := $(filter-out -mcmodel=kernel,$(KBUILD_CFLAGS_32)) KBUILD_CFLAGS_32 := $(filter-out -fno-pic,$(KBUILD_CFLAGS_32)) KBUILD_CFLAGS_32 := $(filter-out -mfentry,$(KBUILD_CFLAGS_32)) KBUILD_CFLAGS_32 := $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS_32)) +KBUILD_CFLAGS_32 := $(filter-out $(RETPOLINE_CFLAGS),$(KBUILD_CFLAGS_32)) KBUILD_CFLAGS_32 += -m32 -msoft-float -mregparm=0 -fpic KBUILD_CFLAGS_32 += $(call cc-option, -fno-stack-protector) KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls) KBUILD_CFLAGS_32 += -fno-omit-frame-pointer KBUILD_CFLAGS_32 += -DDISABLE_BRANCH_PROFILING +KBUILD_CFLAGS_32 += $(RETPOLINE_VDSO_CFLAGS) $(obj)/vdso32.so.dbg: KBUILD_CFLAGS = $(KBUILD_CFLAGS_32) $(obj)/vdso32.so.dbg: FORCE \

5 years, 7 months

1
0
0 0

[PATCH] x86/vdso: fix lsl operand order

by Samuel Neves

In the __getcpu function, lsl was using the wrong target and destination registers. Luckily, the compiler tends to choose %eax for both variables, so it has been working so far. Cc: x86(a)kernel.org Cc: stable(a)vger.kernel.org Signed-off-by: Samuel Neves <sneves(a)dei.uc.pt> --- arch/x86/include/asm/vgtod.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/vgtod.h b/arch/x86/include/asm/vgtod.h index fb856c9f0449..53748541c487 100644 --- a/arch/x86/include/asm/vgtod.h +++ b/arch/x86/include/asm/vgtod.h @@ -93,7 +93,7 @@ static inline unsigned int __getcpu(void) * * If RDPID is available, use it. */ - alternative_io ("lsl %[p],%[seg]", + alternative_io ("lsl %[seg],%[p]", ".byte 0xf3,0x0f,0xc7,0xf8", /* RDPID %eax/rax */ X86_FEATURE_RDPID, [p] "=a" (p), [seg] "r" (__PER_CPU_SEG)); -- 2.17.1

5 years, 7 months

2
1
0 0

[PATCH] ext4: avoid arithemetic overflow that can trigger a BUG

by Theodore Ts'o

A maliciously crafted file system can cause an overflow when the results of a 64-bit calculation is stored into a 32-bit length parameter. https://bugzilla.kernel.org/show_bug.cgi?id=200623 Signed-off-by: Theodore Ts'o <tytso(a)mit.edu> Reported-by: Wen Xu <wen.xu(a)gatech.edu> Cc: stable(a)vger.kernel.org --- fs/ext4/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c index 8f6ad7667974..1134c3473673 100644 --- a/fs/ext4/inode.c +++ b/fs/ext4/inode.c @@ -3414,6 +3414,7 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length, unsigned int blkbits = inode->i_blkbits; unsigned long first_block = offset >> blkbits; unsigned long last_block = (offset + length - 1) >> blkbits; + unsigned long len; struct ext4_map_blocks map; bool delalloc = false; int ret; @@ -3434,7 +3435,8 @@ static int ext4_iomap_begin(struct inode *inode, loff_t offset, loff_t length, } map.m_lblk = first_block; - map.m_len = last_block - first_block + 1; + len = last_block - first_block + 1; + map.m_len = (len < UINT_MAX) ? len : UINT_MAX; if (flags & IOMAP_REPORT) { ret = ext4_map_blocks(NULL, inode, &map, 0); -- 2.18.0.rc0

5 years, 7 months

4
5
0 0

[PATCH] x86/vdso: fix lsl operand order

by Samuel Neves

In the __getcpu function, lsl was using the wrong target and destination registers. Luckily, the compiler tends to choose %eax for both variables, so it has been working so far. Signed-off-by: Samuel Neves <sneves(a)dei.uc.pt> --- arch/x86/include/asm/vgtod.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/x86/include/asm/vgtod.h b/arch/x86/include/asm/vgtod.h index fb856c9f0449..53748541c487 100644 --- a/arch/x86/include/asm/vgtod.h +++ b/arch/x86/include/asm/vgtod.h @@ -93,7 +93,7 @@ static inline unsigned int __getcpu(void) * * If RDPID is available, use it. */ - alternative_io ("lsl %[p],%[seg]", + alternative_io ("lsl %[seg],%[p]", ".byte 0xf3,0x0f,0xc7,0xf8", /* RDPID %eax/rax */ X86_FEATURE_RDPID, [p] "=a" (p), [seg] "r" (__PER_CPU_SEG)); -- 2.17.1

5 years, 7 months

2
1
0 0

[PATCH v4.14.y] tun: fix use after free for ptr_ring

by Zubin Mithra

From: Jason Wang <jasowang(a)redhat.com> commit b196d88aba8ac72b775137854121097f4c4c6862 upstream. We used to initialize ptr_ring during TUNSETIFF, this is because its size depends on the tx_queue_len of netdevice. And we try to clean it up when socket were detached from netdevice. A race were spotted when trying to do uninit during a read which will lead a use after free for pointer ring. Solving this by always initialize a zero size ptr_ring in open() and do resizing during TUNSETIFF, and then we can safely do cleanup during close(). With this, there's no need for the workaround that was introduced by commit 4df0bfc79904 ("tun: fix a memory leak for tfile->tx_array"). Backport Note :- This is a backport of following 2 upstream patches(the second fixes the first). b196d88aba ("tun: fix use after free for ptr_ring") 7063efd33b ("tuntap: fix use after free during release") Comparison with the upstream patch: [1] A "semantic revert" of the changes made in 4df0bfc799("tun: fix a memory leak for tfile->tx_array"). 4df0bfc799 was applied upstream, and then skb array was changed to use ptr_ring. The upstream fix then removes the changes introduced by 4df0bfc799. This backport does the same; "revert" the changes made by 4df0bfc799. [2] xdp_rxq_info_unreg() being called in relevant locations As xdp_rxq_info related patches are not present in 4.14, these changes are not needed in the backport. [3] An instance of ptr_ring_init needs to be replaced by skb_array_init. [4] ptr_ring_cleanup needs to be replaced by skb_array_cleanup. b196d88ab places the cleanup function in tun_chr_close() only to later move it into __tun_detach in upstream commit 7063efd33bb("tuntap: fix use after free during release"). So place skb_array_cleanup in __tun_detach. Reported-by: syzbot+e8b902c3c3fadf0a9dba(a)syzkaller.appspotmail.com Cc: Eric Dumazet <eric.dumazet(a)gmail.com> Cc: Cong Wang <xiyou.wangcong(a)gmail.com> Cc: Michael S. Tsirkin <mst(a)redhat.com> Fixes: 1576d9860599 ("tun: switch to use skb array for tx") Signed-off-by: Jason Wang <jasowang(a)redhat.com> Acked-by: Michael S. Tsirkin <mst(a)redhat.com> Signed-off-by: David S. Miller <davem(a)davemloft.net> Signed-off-by: Zubin Mithra <zsm(a)chromium.org> --- drivers/net/tun.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index cb17ffadfc30..e0baea2dfd3c 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -534,14 +534,6 @@ static void tun_queue_purge(struct tun_file *tfile) skb_queue_purge(&tfile->sk.sk_error_queue); } -static void tun_cleanup_tx_array(struct tun_file *tfile) -{ - if (tfile->tx_array.ring.queue) { - skb_array_cleanup(&tfile->tx_array); - memset(&tfile->tx_array, 0, sizeof(tfile->tx_array)); - } -} - static void __tun_detach(struct tun_file *tfile, bool clean) { struct tun_file *ntfile; @@ -583,7 +575,7 @@ static void __tun_detach(struct tun_file *tfile, bool clean) tun->dev->reg_state == NETREG_REGISTERED) unregister_netdevice(tun->dev); } - tun_cleanup_tx_array(tfile); + skb_array_cleanup(&tfile->tx_array); sock_put(&tfile->sk); } } @@ -623,13 +615,11 @@ static void tun_detach_all(struct net_device *dev) /* Drop read queue */ tun_queue_purge(tfile); sock_put(&tfile->sk); - tun_cleanup_tx_array(tfile); } list_for_each_entry_safe(tfile, tmp, &tun->disabled, next) { tun_enable_queue(tfile); tun_queue_purge(tfile); sock_put(&tfile->sk); - tun_cleanup_tx_array(tfile); } BUG_ON(tun->numdisabled != 0); @@ -675,7 +665,7 @@ static int tun_attach(struct tun_struct *tun, struct file *file, bool skip_filte } if (!tfile->detached && - skb_array_init(&tfile->tx_array, dev->tx_queue_len, GFP_KERNEL)) { + skb_array_resize(&tfile->tx_array, dev->tx_queue_len, GFP_KERNEL)) { err = -ENOMEM; goto out; } @@ -2624,6 +2614,11 @@ static int tun_chr_open(struct inode *inode, struct file * file) &tun_proto, 0); if (!tfile) return -ENOMEM; + if (skb_array_init(&tfile->tx_array, 0, GFP_KERNEL)) { + sk_free(&tfile->sk); + return -ENOMEM; + } + RCU_INIT_POINTER(tfile->tun, NULL); tfile->flags = 0; tfile->ifindex = 0; @@ -2644,8 +2639,6 @@ static int tun_chr_open(struct inode *inode, struct file * file) sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); - memset(&tfile->tx_array, 0, sizeof(tfile->tx_array)); - return 0; } -- 2.19.0.rc0.228.g281dcd1b4d0-goog

5 years, 7 months

2
1
0 0

2024

2023

2022

2021

2020

2019

2018

2017

Linux-stable-mirror September 2018