1. 30 Oct, 2020 4 commits
    • Pablo Neira Ayuso's avatar
      netfilter: nf_tables: missing validation from the abort path · c0391b6a
      Pablo Neira Ayuso authored
      If userspace does not include the trailing end of batch message, then
      nfnetlink aborts the transaction. This allows to check that ruleset
      updates trigger no errors.
      
      After this patch, invoking this command from the prerouting chain:
      
       # nft -c add rule x y fib saddr . oif type local
      
      fails since oif is not supported there.
      
      This patch fixes the lack of rule validation from the abort/check path
      to catch configuration errors such as the one above.
      
      Fixes: a654de8f ("netfilter: nf_tables: fix chain dependency validation")
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      c0391b6a
    • Jason A. Donenfeld's avatar
      netfilter: use actual socket sk rather than skb sk when routing harder · 46d6c5ae
      Jason A. Donenfeld authored
      If netfilter changes the packet mark when mangling, the packet is
      rerouted using the route_me_harder set of functions. Prior to this
      commit, there's one big difference between route_me_harder and the
      ordinary initial routing functions, described in the comment above
      __ip_queue_xmit():
      
         /* Note: skb->sk can be different from sk, in case of tunnels */
         int __ip_queue_xmit(struct sock *sk, struct sk_buff *skb, struct flowi *fl,
      
      That function goes on to correctly make use of sk->sk_bound_dev_if,
      rather than skb->sk->sk_bound_dev_if. And indeed the comment is true: a
      tunnel will receive a packet in ndo_start_xmit with an initial skb->sk.
      It will make some transformations to that packet, and then it will send
      the encapsulated packet out of a *new* socket. That new socket will
      basically always have a different sk_bound_dev_if (otherwise there'd be
      a routing loop). So for the purposes of routing the encapsulated packet,
      the routing information as it pertains to the socket should come from
      that socket's sk, rather than the packet's original skb->sk. For that
      reason __ip_queue_xmit() and related functions all do the right thing.
      
      One might argue that all tunnels should just call skb_orphan(skb) before
      transmitting the encapsulated packet into the new socket. But tunnels do
      *not* do this -- and this is wisely avoided in skb_scrub_packet() too --
      because features like TSQ rely on skb->destructor() being called when
      that buffer space is truely available again. Calling skb_orphan(skb) too
      early would result in buffers filling up unnecessarily and accounting
      info being all wrong. Instead, additional routing must take into account
      the new sk, just as __ip_queue_xmit() notes.
      
      So, this commit addresses the problem by fishing the correct sk out of
      state->sk -- it's already set properly in the call to nf_hook() in
      __ip_local_out(), which receives the sk as part of its normal
      functionality. So we make sure to plumb state->sk through the various
      route_me_harder functions, and then make correct use of it following the
      example of __ip_queue_xmit().
      
      Fixes: 1da177e4 ("Linux-2.6.12-rc2")
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Reviewed-by: default avatarFlorian Westphal <fw@strlen.de>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      46d6c5ae
    • Jason A. Donenfeld's avatar
      wireguard: selftests: check that route_me_harder packets use the right sk · af8afcf1
      Jason A. Donenfeld authored
      If netfilter changes the packet mark, the packet is rerouted. The
      ip_route_me_harder family of functions fails to use the right sk, opting
      to instead use skb->sk, resulting in a routing loop when used with
      tunnels. With the next change fixing this issue in netfilter, test for
      the relevant condition inside our test suite, since wireguard was where
      the bug was discovered.
      Reported-by: default avatarChen Minqiang <ptpt52@gmail.com>
      Signed-off-by: default avatarJason A. Donenfeld <Jason@zx2c4.com>
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      af8afcf1
    • Pablo Neira Ayuso's avatar
      netfilter: nftables: fix netlink report logic in flowtable and genid · dceababa
      Pablo Neira Ayuso authored
      The netlink report should be sent regardless the available listeners.
      
      Fixes: 84d7fce6 ("netfilter: nf_tables: export rule-set generation ID")
      Fixes: 3b49e2e9 ("netfilter: nf_tables: add flow table netlink frontend")
      Signed-off-by: default avatarPablo Neira Ayuso <pablo@netfilter.org>
      dceababa
  2. 29 Oct, 2020 19 commits
    • Linus Torvalds's avatar
      Merge tag 'fallthrough-fixes-clang-5.10-rc2' of... · 07e08873
      Linus Torvalds authored
      Merge tag 'fallthrough-fixes-clang-5.10-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux
      
      Pull fallthrough fix from Gustavo A. R. Silva:
       "This fixes a ton of fall-through warnings when building with Clang
        12.0.0 and -Wimplicit-fallthrough"
      
      * tag 'fallthrough-fixes-clang-5.10-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/gustavoars/linux:
        include: jhash/signal: Fix fall-through warnings for Clang
      07e08873
    • Linus Torvalds's avatar
      Merge tag 'net-5.10-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net · 934291ff
      Linus Torvalds authored
      Pull networking fixes from Jakub Kicinski:
       "Current release regressions:
      
         - r8169: fix forced threading conflicting with other shared
           interrupts; we tried to fix the use of raise_softirq_irqoff from an
           IRQ handler on RT by forcing hard irqs, but this driver shares
           legacy PCI IRQs so drop the _irqoff() instead
      
         - tipc: fix memory leak caused by a recent syzbot report fix to
           tipc_buf_append()
      
        Current release - bugs in new features:
      
         - devlink: Unlock on error in dumpit() and fix some error codes
      
         - net/smc: fix null pointer dereference in smc_listen_decline()
      
        Previous release - regressions:
      
         - tcp: Prevent low rmem stalls with SO_RCVLOWAT.
      
         - net: protect tcf_block_unbind with block lock
      
         - ibmveth: Fix use of ibmveth in a bridge; the self-imposed filtering
           to only send legal frames to the hypervisor was too strict
      
         - net: hns3: Clear the CMDQ registers before unmapping BAR region;
           incorrect cleanup order was leading to a crash
      
         - bnxt_en - handful of fixes to fixes:
            - Send HWRM_FUNC_RESET fw command unconditionally, even if there
              are PCIe errors being reported
            - Check abort error state in bnxt_open_nic().
            - Invoke cancel_delayed_work_sync() for PFs also.
            - Fix regression in workqueue cleanup logic in bnxt_remove_one().
      
         - mlxsw: Only advertise link modes supported by both driver and
           device, after removal of 56G support from the driver 56G was not
           cleared from advertised modes
      
         - net/smc: fix suppressed return code
      
        Previous release - always broken:
      
         - netem: fix zero division in tabledist, caused by integer overflow
      
         - bnxt_en: Re-write PCI BARs after PCI fatal error.
      
         - cxgb4: set up filter action after rewrites
      
         - net: ipa: command payloads already mapped
      
        Misc:
      
         - s390/ism: fix incorrect system EID, it's okay to change since it
           was added in current release
      
         - vsock: use ns_capable_noaudit() on socket create to suppress false
           positive audit messages"
      
      * tag 'net-5.10-rc2' of git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net: (36 commits)
        r8169: fix issue with forced threading in combination with shared interrupts
        netem: fix zero division in tabledist
        ibmvnic: fix ibmvnic_set_mac
        mptcp: add missing memory scheduling in the rx path
        tipc: fix memory leak caused by tipc_buf_append()
        gtp: fix an use-before-init in gtp_newlink()
        net: protect tcf_block_unbind with block lock
        ibmveth: Fix use of ibmveth in a bridge.
        net/sched: act_mpls: Add softdep on mpls_gso.ko
        ravb: Fix bit fields checking in ravb_hwtstamp_get()
        devlink: Unlock on error in dumpit()
        devlink: Fix some error codes
        chelsio/chtls: fix memory leaks in CPL handlers
        chelsio/chtls: fix deadlock issue
        net: hns3: Clear the CMDQ registers before unmapping BAR region
        bnxt_en: Send HWRM_FUNC_RESET fw command unconditionally.
        bnxt_en: Check abort error state in bnxt_open_nic().
        bnxt_en: Re-write PCI BARs after PCI fatal error.
        bnxt_en: Invoke cancel_delayed_work_sync() for PFs also.
        bnxt_en: Fix regression in workqueue cleanup logic in bnxt_remove_one().
        ...
      934291ff
    • Linus Torvalds's avatar
      Merge tag 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma · b9c0f4bd
      Linus Torvalds authored
      Pull rdma fixes from Jason Gunthorpe:
       "The good news is people are testing rc1 in the RDMA world - the bad
        news is testing of the for-next area is not as good as I had hoped, as
        we really should have caught at least the rdma_connect_locked() issue
        before now.
      
        Notable merge window regressions that didn't get caught/fixed in time
        for rc1:
      
         - Fix in kernel users of rxe, they were broken by the rapid fix to
           undo the uABI breakage in rxe from another patch
      
         - EFA userspace needs to read the GID table but was broken with the
           new GID table logic
      
         - Fix user triggerable deadlock in mlx5 using devlink reload
      
         - Fix deadlock in several ULPs using rdma_connect from the CM handler
           callbacks
      
         - Memory leak in qedr"
      
      * tag 'for-linus' of git://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma:
        RDMA/qedr: Fix memory leak in iWARP CM
        RDMA: Add rdma_connect_locked()
        RDMA/uverbs: Fix false error in query gid IOCTL
        RDMA/mlx5: Fix devlink deadlock on net namespace deletion
        RDMA/rxe: Fix small problem in network_type patch
      b9c0f4bd
    • Heiner Kallweit's avatar
      r8169: fix issue with forced threading in combination with shared interrupts · 2734a24e
      Heiner Kallweit authored
      As reported by Serge flag IRQF_NO_THREAD causes an error if the
      interrupt is actually shared and the other driver(s) don't have this
      flag set. This situation can occur if a PCI(e) legacy interrupt is
      used in combination with forced threading.
      There's no good way to deal with this properly, therefore we have to
      remove flag IRQF_NO_THREAD. For fixing the original forced threading
      issue switch to napi_schedule().
      
      Fixes: 424a646e ("r8169: fix operation under forced interrupt threading")
      Link: https://www.spinics.net/lists/netdev/msg694960.htmlReported-by: default avatarSerge Belyshev <belyshev@depni.sinp.msu.ru>
      Signed-off-by: default avatarHeiner Kallweit <hkallweit1@gmail.com>
      Tested-by: default avatarSerge Belyshev <belyshev@depni.sinp.msu.ru>
      Link: https://lore.kernel.org/r/b5b53bfe-35ac-3768-85bf-74d1290cf394@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      2734a24e
    • Aleksandr Nogikh's avatar
      netem: fix zero division in tabledist · eadd1bef
      Aleksandr Nogikh authored
      Currently it is possible to craft a special netlink RTM_NEWQDISC
      command that can result in jitter being equal to 0x80000000. It is
      enough to set the 32 bit jitter to 0x02000000 (it will later be
      multiplied by 2^6) or just set the 64 bit jitter via
      TCA_NETEM_JITTER64. This causes an overflow during the generation of
      uniformly distributed numbers in tabledist(), which in turn leads to
      division by zero (sigma != 0, but sigma * 2 is 0).
      
      The related fragment of code needs 32-bit division - see commit
      9b0ed89 ("netem: remove unnecessary 64 bit modulus"), so switching to
      64 bit is not an option.
      
      Fix the issue by keeping the value of jitter within the range that can
      be adequately handled by tabledist() - [0;INT_MAX]. As negative std
      deviation makes no sense, take the absolute value of the passed value
      and cap it at INT_MAX. Inside tabledist(), switch to unsigned 32 bit
      arithmetic in order to prevent overflows.
      
      Fixes: 1da177e4 ("Linux-2.6.12-rc2")
      Signed-off-by: default avatarAleksandr Nogikh <nogikh@google.com>
      Reported-by: syzbot+ec762a6342ad0d3c0d8f@syzkaller.appspotmail.com
      Acked-by: default avatarStephen Hemminger <stephen@networkplumber.org>
      Link: https://lore.kernel.org/r/20201028170731.1383332-1-aleksandrnogikh@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      eadd1bef
    • Lijun Pan's avatar
      ibmvnic: fix ibmvnic_set_mac · 8fc3672a
      Lijun Pan authored
      Jakub Kicinski brought up a concern in ibmvnic_set_mac().
      ibmvnic_set_mac() does this:
      
      	ether_addr_copy(adapter->mac_addr, addr->sa_data);
      	if (adapter->state != VNIC_PROBED)
      		rc = __ibmvnic_set_mac(netdev, addr->sa_data);
      
      So if state == VNIC_PROBED, the user can assign an invalid address to
      adapter->mac_addr, and ibmvnic_set_mac() will still return 0.
      
      The fix is to validate ethernet address at the beginning of
      ibmvnic_set_mac(), and move the ether_addr_copy to
      the case of "adapter->state != VNIC_PROBED".
      
      Fixes: c26eba03 ("ibmvnic: Update reset infrastructure to support tunable parameters")
      Signed-off-by: default avatarLijun Pan <ljp@linux.ibm.com>
      Link: https://lore.kernel.org/r/20201027220456.71450-1-ljp@linux.ibm.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      8fc3672a
    • Paolo Abeni's avatar
      mptcp: add missing memory scheduling in the rx path · 9c3f94e1
      Paolo Abeni authored
      When moving the skbs from the subflow into the msk receive
      queue, we must schedule there the required amount of memory.
      
      Try to borrow the required memory from the subflow, if needed,
      so that we leverage the existing TCP heuristic.
      
      Fixes: 6771bfd9 ("mptcp: update mptcp ack sequence from work queue")
      Signed-off-by: default avatarPaolo Abeni <pabeni@redhat.com>
      Reviewed-by: default avatarMat Martineau <mathew.j.martineau@linux.intel.com>
      Link: https://lore.kernel.org/r/f6143a6193a083574f11b00dbf7b5ad151bc4ff4.1603810630.git.pabeni@redhat.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      9c3f94e1
    • Gustavo A. R. Silva's avatar
      include: jhash/signal: Fix fall-through warnings for Clang · 4169e889
      Gustavo A. R. Silva authored
      In preparation to enable -Wimplicit-fallthrough for Clang, explicitly
      add break statements instead of letting the code fall through to the
      next case.
      
      This patch adds four break statements that, together, fix almost 40,000
      warnings when building Linux 5.10-rc1 with Clang 12.0.0 and this[1] change
      reverted. Notice that in order to enable -Wimplicit-fallthrough for Clang,
      such change[1] is meant to be reverted at some point. So, this patch helps
      to move in that direction.
      
      Something important to mention is that there is currently a discrepancy
      between GCC and Clang when dealing with switch fall-through to empty case
      statements or to cases that only contain a break/continue/return
      statement[2][3][4].
      
      Now that the -Wimplicit-fallthrough option has been globally enabled[5],
      any compiler should really warn on missing either a fallthrough annotation
      or any of the other case-terminating statements (break/continue/return/
      goto) when falling through to the next case statement. Making exceptions
      to this introduces variation in case handling which may continue to lead
      to bugs, misunderstandings, and a general lack of robustness. The point
      of enabling options like -Wimplicit-fallthrough is to prevent human error
      and aid developers in spotting bugs before their code is even built/
      submitted/committed, therefore eliminating classes of bugs. So, in order
      to really accomplish this, we should, and can, move in the direction of
      addressing any error-prone scenarios and get rid of the unintentional
      fallthrough bug-class in the kernel, entirely, even if there is some minor
      redundancy. Better to have explicit case-ending statements than continue to
      have exceptions where one must guess as to the right result. The compiler
      will eliminate any actual redundancy.
      
      [1] commit e2079e93 ("kbuild: Do not enable -Wimplicit-fallthrough for clang for now")
      [2] https://github.com/ClangBuiltLinux/linux/issues/636
      [3] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=91432
      [4] https://godbolt.org/z/xgkvIh
      [5] commit a035d552 ("Makefile: Globally enable fall-through warning")
      Co-developed-by: default avatarKees Cook <keescook@chromium.org>
      Signed-off-by: default avatarKees Cook <keescook@chromium.org>
      Signed-off-by: default avatarGustavo A. R. Silva <gustavoars@kernel.org>
      4169e889
    • Linus Torvalds's avatar
      Merge tag 'afs-fixes-20201029' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs · 598a5976
      Linus Torvalds authored
      Pull AFS fixes from David Howells:
      
       - Fix copy_file_range() to an afs file now returning EINVAL if the
         splice_write file op isn't supplied.
      
       - Fix a deref-before-check in afs_unuse_cell().
      
       - Fix a use-after-free in afs_xattr_get_acl().
      
       - Fix afs to not try to clear PG_writeback when laundering a page.
      
       - Fix afs to take a ref on a page that it sets PG_private on and to
         drop that ref when clearing PG_private. This is done through recently
         added helpers.
      
       - Fix a page leak if write_begin() fails.
      
       - Fix afs_write_begin() to not alter the dirty region info stored in
         page->private, but rather do this in afs_write_end() instead when we
         know what we actually changed.
      
       - Fix afs_invalidatepage() to alter the dirty region info on a page
         when partial page invalidation occurs so that we don't inadvertantly
         include a span of zeros that will get written back if a page gets
         laundered due to a remote 3rd-party induced invalidation.
      
         We mustn't, however, reduce the dirty region if the page has been
         seen to be mapped (ie. we got called through the page_mkwrite vector)
         as the page might still be mapped and we might lose data if the file
         is extended again.
      
       - Fix the dirty region info to have a lower resolution if the size of
         the page is too large for this to be encoded (e.g. powerpc32 with 64K
         pages).
      
         Note that this might not be the ideal way to handle this, since it
         may allow some leakage of undirtied zero bytes to the server's copy
         in the case of a 3rd-party conflict.
      
      To aid the last two fixes, two additional changes:
      
       - Wrap the manipulations of the dirty region info stored in
         page->private into helper functions.
      
       - Alter the encoding of the dirty region so that the region bounds can
         be stored with one fewer bit, making a bit available for the
         indication of mappedness.
      
      * tag 'afs-fixes-20201029' of git://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs:
        afs: Fix dirty-region encoding on ppc32 with 64K pages
        afs: Fix afs_invalidatepage to adjust the dirty region
        afs: Alter dirty range encoding in page->private
        afs: Wrap page->private manipulations in inline functions
        afs: Fix where page->private is set during write
        afs: Fix page leak on afs_write_begin() failure
        afs: Fix to take ref on page when PG_private is set
        afs: Fix afs_launder_page to not clear PG_writeback
        afs: Fix a use after free in afs_xattr_get_acl()
        afs: Fix tracing deref-before-check
        afs: Fix copy_file_range()
      598a5976
    • Tung Nguyen's avatar
      tipc: fix memory leak caused by tipc_buf_append() · ceb1eb2f
      Tung Nguyen authored
      Commit ed42989e ("tipc: fix the skb_unshare() in tipc_buf_append()")
      replaced skb_unshare() with skb_copy() to not reduce the data reference
      counter of the original skb intentionally. This is not the correct
      way to handle the cloned skb because it causes memory leak in 2
      following cases:
       1/ Sending multicast messages via broadcast link
        The original skb list is cloned to the local skb list for local
        destination. After that, the data reference counter of each skb
        in the original list has the value of 2. This causes each skb not
        to be freed after receiving ACK:
        tipc_link_advance_transmq()
        {
         ...
         /* release skb */
         __skb_unlink(skb, &l->transmq);
         kfree_skb(skb); <-- memory exists after being freed
        }
      
       2/ Sending multicast messages via replicast link
        Similar to the above case, each skb cannot be freed after purging
        the skb list:
        tipc_mcast_xmit()
        {
         ...
         __skb_queue_purge(pkts); <-- memory exists after being freed
        }
      
      This commit fixes this issue by using skb_unshare() instead. Besides,
      to avoid use-after-free error reported by KASAN, the pointer to the
      fragment is set to NULL before calling skb_unshare() to make sure that
      the original skb is not freed after freeing the fragment 2 times in
      case skb_unshare() returns NULL.
      
      Fixes: ed42989e ("tipc: fix the skb_unshare() in tipc_buf_append()")
      Acked-by: default avatarJon Maloy <jmaloy@redhat.com>
      Reported-by: default avatarThang Hoang Ngo <thang.h.ngo@dektech.com.au>
      Signed-off-by: default avatarTung Nguyen <tung.q.nguyen@dektech.com.au>
      Reviewed-by: default avatarXin Long <lucien.xin@gmail.com>
      Acked-by: default avatarCong Wang <xiyou.wangcong@gmail.com>
      Link: https://lore.kernel.org/r/20201027032403.1823-1-tung.q.nguyen@dektech.com.auSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      ceb1eb2f
    • Masahiro Fujiwara's avatar
      gtp: fix an use-before-init in gtp_newlink() · 51467431
      Masahiro Fujiwara authored
      *_pdp_find() from gtp_encap_recv() would trigger a crash when a peer
      sends GTP packets while creating new GTP device.
      
      RIP: 0010:gtp1_pdp_find.isra.0+0x68/0x90 [gtp]
      <SNIP>
      Call Trace:
       <IRQ>
       gtp_encap_recv+0xc2/0x2e0 [gtp]
       ? gtp1_pdp_find.isra.0+0x90/0x90 [gtp]
       udp_queue_rcv_one_skb+0x1fe/0x530
       udp_queue_rcv_skb+0x40/0x1b0
       udp_unicast_rcv_skb.isra.0+0x78/0x90
       __udp4_lib_rcv+0x5af/0xc70
       udp_rcv+0x1a/0x20
       ip_protocol_deliver_rcu+0xc5/0x1b0
       ip_local_deliver_finish+0x48/0x50
       ip_local_deliver+0xe5/0xf0
       ? ip_protocol_deliver_rcu+0x1b0/0x1b0
      
      gtp_encap_enable() should be called after gtp_hastable_new() otherwise
      *_pdp_find() will access the uninitialized hash table.
      
      Fixes: 1e3a3abd ("gtp: make GTP sockets in gtp_newlink optional")
      Signed-off-by: default avatarMasahiro Fujiwara <fujiwara.masahiro@gmail.com>
      Link: https://lore.kernel.org/r/20201027114846.3924-1-fujiwara.masahiro@gmail.comSigned-off-by: default avatarJakub Kicinski <kuba@kernel.org>
      51467431
    • Linus Torvalds's avatar
      Merge tag 'ext4_for_linus_fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4 · 58130a6c
      Linus Torvalds authored
      Pull ext4 fixes from Ted Ts'o:
       "Bug fixes for the new ext4 fast commit feature, plus a fix for the
        'data=journal' bug fix.
      
        Also use the generic casefolding support which has now landed in
        fs/libfs.c for 5.10"
      
      * tag 'ext4_for_linus_fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/tytso/ext4:
        ext4: indicate that fast_commit is available via /sys/fs/ext4/feature/...
        ext4: use generic casefolding support
        ext4: do not use extent after put_bh
        ext4: use IS_ERR() for error checking of path
        ext4: fix mmap write protection for data=journal mode
        jbd2: fix a kernel-doc markup
        ext4: use s_mount_flags instead of s_mount_state for fast commit state
        ext4: make num of fast commit blocks configurable
        ext4: properly check for dirty state in ext4_inode_datasync_dirty()
        ext4: fix double locking in ext4_fc_commit_dentry_updates()
      58130a6c
    • David Howells's avatar
      afs: Fix dirty-region encoding on ppc32 with 64K pages · 2d9900f2
      David Howells authored
      The dirty region bounds stored in page->private on an afs page are 15 bits
      on a 32-bit box and can, at most, represent a range of up to 32K within a
      32K page with a resolution of 1 byte.  This is a problem for powerpc32 with
      64K pages enabled.
      
      Further, transparent huge pages may get up to 2M, which will be a problem
      for the afs filesystem on all 32-bit arches in the future.
      
      Fix this by decreasing the resolution.  For the moment, a 64K page will
      have a resolution determined from PAGE_SIZE.  In the future, the page will
      need to be passed in to the helper functions so that the page size can be
      assessed and the resolution determined dynamically.
      
      Note that this might not be the ideal way to handle this, since it may
      allow some leakage of undirtied zero bytes to the server's copy in the case
      of a 3rd-party conflict.  Fixing that would require a separately allocated
      record and is a more complicated fix.
      
      Fixes: 4343d008 ("afs: Get rid of the afs_writeback record")
      Reported-by: default avatarkernel test robot <lkp@intel.com>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarMatthew Wilcox (Oracle) <willy@infradead.org>
      2d9900f2
    • David Howells's avatar
      afs: Fix afs_invalidatepage to adjust the dirty region · f86726a6
      David Howells authored
      Fix afs_invalidatepage() to adjust the dirty region recorded in
      page->private when truncating a page.  If the dirty region is entirely
      removed, then the private data is cleared and the page dirty state is
      cleared.
      
      Without this, if the page is truncated and then expanded again by truncate,
      zeros from the expanded, but no-longer dirty region may get written back to
      the server if the page gets laundered due to a conflicting 3rd-party write.
      
      It mustn't, however, shorten the dirty region of the page if that page is
      still mmapped and has been marked dirty by afs_page_mkwrite(), so a flag is
      stored in page->private to record this.
      
      Fixes: 4343d008 ("afs: Get rid of the afs_writeback record")
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      f86726a6
    • David Howells's avatar
      afs: Alter dirty range encoding in page->private · 65dd2d60
      David Howells authored
      Currently, page->private on an afs page is used to store the range of
      dirtied data within the page, where the range includes the lower bound, but
      excludes the upper bound (e.g. 0-1 is a range covering a single byte).
      
      This, however, requires a superfluous bit for the last-byte bound so that
      on a 4KiB page, it can say 0-4096 to indicate the whole page, the idea
      being that having both numbers the same would indicate an empty range.
      This is unnecessary as the PG_private bit is clear if it's an empty range
      (as is PG_dirty).
      
      Alter the way the dirty range is encoded in page->private such that the
      upper bound is reduced by 1 (e.g. 0-0 is then specified the same single
      byte range mentioned above).
      
      Applying this to both bounds frees up two bits, one of which can be used in
      a future commit.
      
      This allows the afs filesystem to be compiled on ppc32 with 64K pages;
      without this, the following warnings are seen:
      
      ../fs/afs/internal.h: In function 'afs_page_dirty_to':
      ../fs/afs/internal.h:881:15: warning: right shift count >= width of type [-Wshift-count-overflow]
        881 |  return (priv >> __AFS_PAGE_PRIV_SHIFT) & __AFS_PAGE_PRIV_MASK;
            |               ^~
      ../fs/afs/internal.h: In function 'afs_page_dirty':
      ../fs/afs/internal.h:886:28: warning: left shift count >= width of type [-Wshift-count-overflow]
        886 |  return ((unsigned long)to << __AFS_PAGE_PRIV_SHIFT) | from;
            |                            ^~
      
      Fixes: 4343d008 ("afs: Get rid of the afs_writeback record")
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      65dd2d60
    • David Howells's avatar
      afs: Wrap page->private manipulations in inline functions · 185f0c70
      David Howells authored
      The afs filesystem uses page->private to store the dirty range within a
      page such that in the event of a conflicting 3rd-party write to the server,
      we write back just the bits that got changed locally.
      
      However, there are a couple of problems with this:
      
       (1) I need a bit to note if the page might be mapped so that partial
           invalidation doesn't shrink the range.
      
       (2) There aren't necessarily sufficient bits to store the entire range of
           data altered (say it's a 32-bit system with 64KiB pages or transparent
           huge pages are in use).
      
      So wrap the accesses in inline functions so that future commits can change
      how this works.
      
      Also move them out of the tracing header into the in-directory header.
      There's not really any need for them to be in the tracing header.
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      185f0c70
    • David Howells's avatar
      afs: Fix where page->private is set during write · f792e3ac
      David Howells authored
      In afs, page->private is set to indicate the dirty region of a page.  This
      is done in afs_write_begin(), but that can't take account of whether the
      copy into the page actually worked.
      
      Fix this by moving the change of page->private into afs_write_end().
      
      Fixes: 4343d008 ("afs: Get rid of the afs_writeback record")
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      f792e3ac
    • David Howells's avatar
      afs: Fix page leak on afs_write_begin() failure · 21db2cdc
      David Howells authored
      Fix the leak of the target page in afs_write_begin() when it fails.
      
      Fixes: 15b4650e ("afs: convert to new aops")
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      cc: Nick Piggin <npiggin@gmail.com>
      21db2cdc
    • David Howells's avatar
      afs: Fix to take ref on page when PG_private is set · fa04a40b
      David Howells authored
      Fix afs to take a ref on a page when it sets PG_private on it and to drop
      the ref when removing the flag.
      
      Note that in afs_write_begin(), a lot of the time, PG_private is already
      set on a page to which we're going to add some data.  In such a case, we
      leave the bit set and mustn't increment the page count.
      
      As suggested by Matthew Wilcox, use attach/detach_page_private() where
      possible.
      
      Fixes: 31143d5d ("AFS: implement basic file write support")
      Reported-by: default avatarMatthew Wilcox (Oracle) <willy@infradead.org>
      Signed-off-by: default avatarDavid Howells <dhowells@redhat.com>
      Reviewed-by: default avatarMatthew Wilcox (Oracle) <willy@infradead.org>
      fa04a40b
  3. 28 Oct, 2020 17 commits