On 2020-05-22 01:46, Robin Murphy wrote:
> On 2020-05-21 12:30, Prakash Gupta wrote:
>> Limit the iova size while freeing based on unmapped size. In absence
>> of
>> this even with unmap failure, invalid iova is pushed to iova rcache
>> and
>> subsequently can cause panic while rcache magazine is freed.
>
> Can you elaborate on that panic?
>
We have seen couple of stability issues around this.
Below is one such example:
kernel BUG at kernel/msm-4.19/drivers/iommu/iova.c:904!
iova_magazine_free_pfns
iova_rcache_insert
free_iova_fast
__iommu_unmap_page
iommu_dma_unmap_page
It turned out an iova pfn 0 got into iova_rcache. One possibility I see
is
where client unmap with invalid dma_addr. The unmap call will fail and
warn on
and still try to free iova. This will cause invalid pfn to be inserted
into
rcache. As and when the magazine with invalid pfn will be freed
private_find_iova() will return NULL for invalid iova and meet bug
condition.
>> Signed-off-by: Prakash Gupta <guptap(a)codeaurora.org>
>>
>> :100644 100644 4959f5df21bd 098f7d377e04 M drivers/iommu/dma-iommu.c
>>
>> diff --git a/drivers/iommu/dma-iommu.c b/drivers/iommu/dma-iommu.c
>> index 4959f5df21bd..098f7d377e04 100644
>> --- a/drivers/iommu/dma-iommu.c
>> +++ b/drivers/iommu/dma-iommu.c
>> @@ -472,7 +472,8 @@ static void __iommu_dma_unmap(struct device *dev,
>> dma_addr_t dma_addr,
>> if (!cookie->fq_domain)
>> iommu_tlb_sync(domain, &iotlb_gather);
>> - iommu_dma_free_iova(cookie, dma_addr, size);
>> + if (unmapped)
>> + iommu_dma_free_iova(cookie, dma_addr, unmapped);
>
> Frankly, if any part of the unmap fails then things have gone
> catastrophically wrong already, but either way this isn't right. The
> IOVA API doesn't support partial freeing - an IOVA *must* be freed
> with its original size, or not freed at all, otherwise it will corrupt
> the state of the rcaches and risk a cascade of further misbehaviour
> for future callers.
>
I agree, we shouldn't be freeing the partial iova. Instead just making
sure if unmap was successful should be sufficient before freeing iova.
So change
can instead be something like this:
- iommu_dma_free_iova(cookie, dma_addr, size);
+ if (unmapped)
+ iommu_dma_free_iova(cookie, dma_addr, size);
> TBH my gut feeling here is that you're really just trying to treat a
> symptom of another bug elsewhere, namely some driver calling
> dma_unmap_* or dma_free_* with the wrong address or size in the first
> place.
>
This condition would arise only if driver calling dma_unmap/free_* with
0
iova_pfn. This will be flagged with a warning during unmap but will
trigger
panic later on while doing unrelated dma_map/unmap_*. If unmapped has
already
failed for invalid iova, there is no reason we should consider this as
valid
iova and free. This part should be fixed.
On 2020-05-22 00:19, Andrew Morton wrote:
> I think we need a cc:stable here?
>
Added now.
Thanks,
Prakash
> Additionally, there's no point copying param->string in
> exfat_parse_param() - just steal it, leaving NULL in param->string.
> That's independent from the leak or fix thereof - it's simply
> avoiding an extra copy.
I find it clearer to provide such a source code adjustment
by a separate update step.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Doc…
Please move it into another patch.
…
> +++ b/fs/exfat/super.c
…
> @@ -686,7 +685,12 @@ static int exfat_get_tree(struct fs_context *fc)
>
> static void exfat_free(struct fs_context *fc)
> {
> - kfree(fc->s_fs_info);
> + struct exfat_sb_info *sbi = fc->s_fs_info;
> +
> + if (sbi) {
> + exfat_free_iocharset(sbi);
> + kfree(sbi);
> + }
> }
…
Can it be helpful to annotate the added check according to branch prediction?
Are valid pointers likely at this place?
Regards,
Markus
This is the start of the stable review cycle for the 4.4.226 release.
There are 47 patches in this series, all will be posted as a response
to this one. If anyone has any issues with these being applied, please
let me know.
Responses should be made by Thu, 04 Jun 2020 09:57:12 +0000.
Anything received after that time might be too late.
The whole patch series can be found in one patch at:
https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.4.226-rc…
or in the git tree and branch at:
git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git linux-4.4.y
and the diffstat can be found below.
thanks,
greg k-h
-------------
Pseudo-Shortlog of commits:
Greg Kroah-Hartman <gregkh(a)linuxfoundation.org>
Linux 4.4.226-rc2
Benjamin Block <bblock(a)linux.ibm.com>
scsi: zfcp: fix request object use-after-free in send path causing wrong traces
Aaron Conole <aconole(a)redhat.com>
printk: help pr_debug and pr_devel to optimize out arguments
Ben Hutchings <ben.hutchings(a)codethink.co.uk>
drm/msm: Fix possible null dereference on failure of get_pages()
Guoqing Jiang <gqjiang(a)suse.com>
sc16is7xx: move label 'err_spi' to correct section
Michal Marek <mmarek(a)suse.com>
asm-prototypes: Clear any CPP defines before declaring the functions
Liviu Dudau <liviu(a)dudau.co.uk>
mm/vmalloc.c: don't dereference possible NULL pointer in __vunmap()
Roopa Prabhu <roopa(a)cumulusnetworks.com>
net: rtnl_configure_link: fix dev flags changes arg to __dev_notify_flags
Sudip Mukherjee <sudip(a)vectorindia.org>
mac80211: fix memory leak
Yoshihiro Shimoda <yoshihiro.shimoda.uh(a)renesas.com>
usb: renesas_usbhs: gadget: fix spin_lock_init() for &uep->lock
Thomas Gleixner <tglx(a)linutronix.de>
genirq/generic_pending: Do not lose pending affinity update
Matt Roper <matthew.d.roper(a)intel.com>
drm/fb-helper: Use proper plane mask for fb cleanup
Konstantin Khlebnikov <khlebnikov(a)yandex-team.ru>
mm: remove VM_BUG_ON(PageSlab()) from page_mapcount()
Pablo Neira Ayuso <pablo(a)netfilter.org>
netfilter: nf_conntrack_pptp: fix compilation warning with W=1 build
Qiushi Wu <wu000273(a)umn.edu>
bonding: Fix reference count leak in bond_sysfs_slave_add.
Qiushi Wu <wu000273(a)umn.edu>
qlcnic: fix missing release in qlcnic_83xx_interrupt_test.
Pablo Neira Ayuso <pablo(a)netfilter.org>
netfilter: nf_conntrack_pptp: prevent buffer overflows in debug code
Phil Sutter <phil(a)nwl.cc>
netfilter: ipset: Fix subcounter update skip
Michael Braun <michael-dev(a)fami-braun.de>
netfilter: nft_reject_bridge: enable reject with bridge vlan
Xin Long <lucien.xin(a)gmail.com>
ip_vti: receive ipip packet by calling ip_tunnel_rcv
Jeremy Sowden <jeremy(a)azazel.net>
vti4: eliminated some duplicate code.
Xin Long <lucien.xin(a)gmail.com>
xfrm: fix a NULL-ptr deref in xfrm_local_error
Xin Long <lucien.xin(a)gmail.com>
xfrm: fix a warning in xfrm_policy_insert_list
Xin Long <lucien.xin(a)gmail.com>
xfrm: allow to accept packets with ipv6 NEXTHDR_HOP in xfrm_input
Alexander Dahl <post(a)lespocky.de>
x86/dma: Fix max PFN arithmetic overflow on 32 bit systems
Helge Deller <deller(a)gmx.de>
parisc: Fix kernel panic in mem_init()
Qiushi Wu <wu000273(a)umn.edu>
iommu: Fix reference count leak in iommu_group_alloc.
Arnd Bergmann <arnd(a)arndb.de>
include/asm-generic/topology.h: guard cpumask_of_node() macro argument
Alexander Potapenko <glider(a)google.com>
fs/binfmt_elf.c: allocate initialized memory in fill_thread_core_info()
Eric W. Biederman <ebiederm(a)xmission.com>
exec: Always set cap_ambient in cap_bprm_set_creds
Chris Chiu <chiu(a)endlessm.com>
ALSA: usb-audio: mixer: volume quirk for ESS Technology Asus USB DAC
Changming Liu <liu.changm(a)northeastern.edu>
ALSA: hwdep: fix a left shifting 1 by 31 UB bug
Kaike Wan <kaike.wan(a)intel.com>
IB/qib: Call kobject_put() when kobject_init_and_add() fails
Kevin Locke <kevin(a)kevinlocke.name>
Input: i8042 - add ThinkPad S230u to i8042 reset list
Łukasz Patron <priv.luk(a)gmail.com>
Input: xpad - add custom init packet for Xbox One S controllers
Brendan Shanks <bshanks(a)codeweavers.com>
Input: evdev - call input_flush_device() on release(), not flush()
James Hilliard <james.hilliard1(a)gmail.com>
Input: usbtouchscreen - add support for BonXeon TP
Steve French <stfrench(a)microsoft.com>
cifs: Fix null pointer check in cifs_read
Masahiro Yamada <masahiroy(a)kernel.org>
usb: gadget: legacy: fix redundant initialization warnings
Lei Xue <carmark.dlut(a)gmail.com>
cachefiles: Fix race between read_waiter and read_copier involving op->to_do
Kalderon, Michal <Michal.Kalderon(a)cavium.com>
IB/cma: Fix reference count leak when no ipv4 addresses are set
Dmitry V. Levin <ldv(a)altlinux.org>
uapi: fix linux/if_pppol2tp.h userspace compilation errors
Qiushi Wu <wu000273(a)umn.edu>
net/mlx4_core: fix a memory leak bug.
Qiushi Wu <wu000273(a)umn.edu>
net: sun: fix missing release regions in cas_init_one().
Moshe Shemesh <moshe(a)mellanox.com>
net/mlx5: Add command entry handling completion
Jere Leppänen <jere.leppanen(a)nokia.com>
sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed
Yuqi Jin <jinyuqi(a)huawei.com>
net: revert "net: get rid of an signed integer overflow in ip_idents_reserve()"
Eric Dumazet <edumazet(a)google.com>
ax25: fix setsockopt(SO_BINDTODEVICE)
-------------
Diffstat:
Makefile | 4 +-
arch/parisc/mm/init.c | 2 +-
arch/x86/include/asm/dma.h | 2 +-
drivers/gpu/drm/drm_fb_helper.c | 2 +-
drivers/gpu/drm/msm/msm_gem.c | 20 +++---
drivers/infiniband/hw/qib/qib_sysfs.c | 9 +--
drivers/input/evdev.c | 19 ++----
drivers/input/joystick/xpad.c | 12 ++++
drivers/input/serio/i8042-x86ia64io.h | 7 ++
drivers/input/touchscreen/usbtouchscreen.c | 1 +
drivers/iommu/iommu.c | 2 +-
drivers/net/bonding/bond_sysfs_slave.c | 4 +-
drivers/net/ethernet/mellanox/mlx4/fw.c | 2 +-
drivers/net/ethernet/mellanox/mlx5/core/cmd.c | 15 +++++
.../net/ethernet/qlogic/qlcnic/qlcnic_83xx_hw.c | 4 +-
drivers/net/ethernet/sun/cassini.c | 3 +-
drivers/s390/scsi/zfcp_fsf.c | 10 ++-
drivers/tty/serial/sc16is7xx.c | 2 +
drivers/usb/gadget/legacy/inode.c | 3 +-
drivers/usb/renesas_usbhs/mod_gadget.c | 2 +-
fs/binfmt_elf.c | 2 +-
fs/cachefiles/rdwr.c | 2 +-
fs/cifs/file.c | 2 +-
include/asm-generic/asm-prototypes.h | 6 ++
include/asm-generic/topology.h | 2 +-
include/linux/mlx5/driver.h | 1 +
include/linux/mm.h | 1 -
include/linux/netfilter/nf_conntrack_pptp.h | 2 +-
include/linux/printk.h | 12 ++--
include/rdma/ib_addr.h | 6 +-
include/uapi/linux/l2tp.h | 7 +-
kernel/irq/migration.c | 26 ++++++--
mm/vmalloc.c | 2 +-
net/ax25/af_ax25.c | 6 +-
net/bridge/netfilter/nft_reject_bridge.c | 6 ++
net/core/rtnetlink.c | 2 +-
net/ipv4/ip_vti.c | 75 ++++++++++++----------
net/ipv4/netfilter/nf_nat_pptp.c | 7 +-
net/ipv4/route.c | 14 ++--
net/mac80211/sta_info.c | 1 +
net/netfilter/ipset/ip_set_list_set.c | 2 +-
net/netfilter/nf_conntrack_pptp.c | 62 ++++++++++--------
net/sctp/sm_statefuns.c | 9 +--
net/xfrm/xfrm_input.c | 2 +-
net/xfrm/xfrm_output.c | 3 +-
net/xfrm/xfrm_policy.c | 7 +-
security/commoncap.c | 1 +
sound/core/hwdep.c | 4 +-
sound/usb/mixer.c | 8 +++
49 files changed, 242 insertions(+), 163 deletions(-)
******************************************
* WARNING: Boot tests are now deprecated *
******************************************
As kernelci.org is expanding its functional testing capabilities, the concept
of boot testing is now deprecated. Boot results are scheduled to be dropped on
*5th June 2020*. The full schedule for boot tests deprecation is available on
this GitHub issue: https://github.com/kernelci/kernelci-backend/issues/238
The new equivalent is the *baseline* test suite which also runs sanity checks
using dmesg and bootrr: https://github.com/kernelci/bootrr
See the *baseline results for this kernel revision* on this page:
https://kernelci.org/test/job/stable-rc/branch/linux-4.9.y/kernel/v4.9.225-…
-------------------------------------------------------------------------------
stable-rc/linux-4.9.y boot: 86 boots: 2 failed, 78 passed with 4 offline, 2 untried/unknown (v4.9.225-56-ga836fd8c024d)
Full Boot Summary: https://kernelci.org/boot/all/job/stable-rc/branch/linux-4.9.y/kernel/v4.9.…
Full Build Summary: https://kernelci.org/build/stable-rc/branch/linux-4.9.y/kernel/v4.9.225-56-…
Tree: stable-rc
Branch: linux-4.9.y
Git Describe: v4.9.225-56-ga836fd8c024d
Git Commit: a836fd8c024d14989c7cbfb91040e805b093f1d7
Git URL: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git
Tested: 54 unique boards, 18 SoC families, 15 builds out of 157
Boot Regressions Detected:
x86_64:
x86_64_defconfig:
gcc-8:
qemu_x86_64:
lab-baylibre: new failure (last pass: v4.9.225-60-g6915714f12d0)
Boot Failures Detected:
arm:
sama5_defconfig:
gcc-8:
at91-sama5d4_xplained: 1 failed lab
x86_64:
x86_64_defconfig:
gcc-8:
qemu_x86_64: 1 failed lab
Offline Platforms:
arm:
exynos_defconfig:
gcc-8
exynos5800-peach-pi: 1 offline lab
multi_v7_defconfig:
gcc-8
exynos5800-peach-pi: 1 offline lab
qcom-apq8064-cm-qs600: 1 offline lab
stih410-b2120: 1 offline lab
---
For more info write to <info(a)kernelci.org>
From: Eric Sandeen <sandeen(a)redhat.com>
commit 2c4306f719b083d17df2963bc761777576b8ad1b upstream.
If xfs_bmap_extents_to_btree fails in a mode where we call
xfs_iroot_realloc(-1) to de-allocate the root, set the
format back to extents.
Otherwise we can assume we can dereference ifp->if_broot
based on the XFS_DINODE_FMT_BTREE format, and crash.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199423
Signed-off-by: Eric Sandeen <sandeen(a)redhat.com>
Reviewed-by: Christoph Hellwig <hch(a)lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong(a)oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong(a)oracle.com>
Signed-off-by: Nobuhiro Iwamatsu (CIP) <nobuhiro1.iwamatsu(a)toshiba.co.jp>
---
fs/xfs/libxfs/xfs_bmap.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 84245d2101828..2b07dadc59167 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -761,12 +761,16 @@ xfs_bmap_extents_to_btree(
*logflagsp = 0;
if ((error = xfs_alloc_vextent(&args))) {
xfs_iroot_realloc(ip, -1, whichfork);
+ ASSERT(ifp->if_broot == NULL);
+ XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return error;
}
if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
xfs_iroot_realloc(ip, -1, whichfork);
+ ASSERT(ifp->if_broot == NULL);
+ XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return -ENOSPC;
}
--
2.27.0.rc0
From: Eric Sandeen <sandeen(a)redhat.com>
commit 2c4306f719b083d17df2963bc761777576b8ad1b upstream.
If xfs_bmap_extents_to_btree fails in a mode where we call
xfs_iroot_realloc(-1) to de-allocate the root, set the
format back to extents.
Otherwise we can assume we can dereference ifp->if_broot
based on the XFS_DINODE_FMT_BTREE format, and crash.
Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199423
Signed-off-by: Eric Sandeen <sandeen(a)redhat.com>
Reviewed-by: Christoph Hellwig <hch(a)lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong(a)oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong(a)oracle.com>
Signed-off-by: Nobuhiro Iwamatsu (CIP) <nobuhiro1.iwamatsu(a)toshiba.co.jp>
---
fs/xfs/libxfs/xfs_bmap.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 9ca8809ee3d0c..e390a7882933c 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -781,6 +781,8 @@ xfs_bmap_extents_to_btree(
*logflagsp = 0;
if ((error = xfs_alloc_vextent(&args))) {
xfs_iroot_realloc(ip, -1, whichfork);
+ ASSERT(ifp->if_broot == NULL);
+ XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return error;
}
@@ -801,6 +803,8 @@ xfs_bmap_extents_to_btree(
}
if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
xfs_iroot_realloc(ip, -1, whichfork);
+ ASSERT(ifp->if_broot == NULL);
+ XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
return -ENOSPC;
}
--
2.26.0