Re: [PATCH] cifs: fix a credits leak for compund commands

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



this does address the problem with the previous version of the patch.
I did some tests on it just now

With your fix test 091 logs the following

[ 1007.477182] run fstests generic/091 at 2018-10-10 00:41:49
[ 1014.991772] server overflowed SMB3 credits


I also still see this oops on test 377 (which may be unrelated to your
patch, but is an oops ...)

[ 1432.339015] run fstests generic/377 at 2018-10-10 00:48:54
[ 1432.522155] CIFS: Attempting to mount //localhost/test
[ 1432.522180] CIFS VFS: mount options mfsymlinks and sfu both enabled
[ 1432.651023] CIFS: Attempting to mount //localhost/scratch
[ 1432.651090] CIFS VFS: mount options mfsymlinks and sfu both enabled
[ 1432.712166] CIFS: Attempting to mount //localhost/scratch
[ 1432.712203] CIFS VFS: mount options mfsymlinks and sfu both enabled
[ 1432.735373] usercopy: Kernel memory exposure attempt detected from
SLUB object 'kmalloc-16' (offset 0, size 30)!
[ 1432.735383] ------------[ cut here ]------------
[ 1432.735384] kernel BUG at mm/usercopy.c:102!
[ 1432.735390] invalid opcode: 0000 [#1] SMP PTI
[ 1432.735394] CPU: 6 PID: 7000 Comm: listxattr Tainted: G
OE     4.19.0-041900rc7-generic #201810071631
[ 1432.735396] Hardware name: LENOVO 20HJS01E00/20HJS01E00, BIOS
N1UET71W (1.45 ) 07/18/2018
[ 1432.735403] RIP: 0010:usercopy_abort+0x7a/0x7c
[ 1432.735406] Code: 0f 45 c6 51 48 89 f9 48 c7 c2 45 9d 50 8e 41 52
48 c7 c6 29 8a 4f 8e 48 c7 c7 10 9e 50 8e 48 0f 45 f2 48 89 c2 e8 0f
03 e6 ff <0f> 0b 4d 89 e0 31 c9 44 89 ea 31 f6 48 c7 c7 79 9d 50 8e e8
6e ff
[ 1432.735409] RSP: 0018:ffffa8df102bfe30 EFLAGS: 00010246
[ 1432.735412] RAX: 0000000000000064 RBX: ffff9dadcddccea0 RCX: 0000000000000000
[ 1432.735414] RDX: 0000000000000000 RSI: ffff9daddf796428 RDI: ffff9daddf796428
[ 1432.735416] RBP: ffffa8df102bfe48 R08: 00000000000b388d R09: 00000000000004ef
[ 1432.735418] R10: 0000000000000004 R11: ffffffff8ed933ed R12: 000000000000001e
[ 1432.735420] R13: 0000000000000001 R14: ffff9dadcddccebe R15: ffff9dad35a62240
[ 1432.735423] FS:  00007fbc0779b740(0000) GS:ffff9daddf780000(0000)
knlGS:0000000000000000
[ 1432.735425] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1432.735427] CR2: 000055d070dd9008 CR3: 000000072ca1e005 CR4: 00000000003606e0
[ 1432.735430] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1432.735431] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1432.735433] Call Trace:
[ 1432.735440]  __check_heap_object+0xdf/0x110
[ 1432.735444]  __check_object_size+0x103/0x18a
[ 1432.735447]  listxattr+0xa6/0xe0
[ 1432.735450]  path_listxattr+0x63/0xb0
[ 1432.735454]  __x64_sys_listxattr+0x1f/0x30
[ 1432.735458]  do_syscall_64+0x5a/0x110
[ 1432.735462]  entry_SYSCALL_64_after_hwframe+0x44/0xa9
[ 1432.735465] RIP: 0033:0x7fbc06ea0839
[ 1432.735468] Code: 00 f3 c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40
00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24
08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 1f f6 2c 00 f7 d8 64 89
01 48
[ 1432.735470] RSP: 002b:00007fffda1e2348 EFLAGS: 00000206 ORIG_RAX:
00000000000000c2
[ 1432.735473] RAX: ffffffffffffffda RBX: 0000000000000009 RCX: 00007fbc06ea0839
[ 1432.735475] RDX: 0000000000000009 RSI: 000055d070dd9260 RDI: 00007fffda1e3a82
[ 1432.735477] RBP: 00007fffda1e2458 R08: 0000000000000000 R09: 000055d06f950b00
[ 1432.735479] R10: 0000000000000002 R11: 0000000000000206 R12: 000055d070dd9260
[ 1432.735481] R13: 00007fffda1e2450 R14: 0000000000000000 R15: 0000000000000000
[ 1432.735484] Modules linked in: md4 nls_utf8 cifs(OE) ccm fscache
vmnet(OE) vmw_vsock_vmci_transport vsock vmw_vmci vmmon(OE)
thunderbolt rfcomm cmac bnep binfmt_misc nls_iso8859_1 arc4 intel_rapl
x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_codec_hdmi
kvm_intel crct10dif_pclmul crc32_pclmul ghash_clmulni_intel pcbc
uvcvideo videobuf2_vmalloc videobuf2_memops videobuf2_v4l2 aesni_intel
snd_hda_codec_realtek videobuf2_common snd_hda_codec_generic
aes_x86_64 crypto_simd cryptd glue_helper snd_seq_dummy videodev media
snd_seq_oss iwlmvm snd_hda_intel btusb intel_cstate intel_rapl_perf
snd_hda_codec btrtl mac80211 btbcm idma64 virt_dma btintel
snd_seq_midi snd_hda_core snd_seq_midi_event bluetooth snd_hwdep
snd_rawmidi ecdh_generic snd_seq iwlwifi snd_pcm snd_seq_device mei_me
thinkpad_acpi
[ 1432.735536]  joydev input_leds rtsx_pci_ms intel_lpss_pci serio_raw
cfg80211 memstick wmi_bmof intel_wmi_thunderbolt nvram mei snd_timer
intel_lpss intel_pch_thermal snd soundcore mac_hid acpi_pad
sch_fq_codel nfsd auth_rpcgss nfs_acl parport_pc lockd grace ppdev lp
sunrpc parport ip_tables x_tables autofs4 btrfs xor zstd_compress
raid6_pq libcrc32c wacom hid_generic usbhid mmc_block i915 nouveau
kvmgt vfio_mdev mdev vfio_iommu_type1 vfio rtsx_pci_sdmmc kvm mxm_wmi
irqbypass ttm i2c_algo_bit drm_kms_helper syscopyarea sysfillrect
sysimgblt fb_sys_fops psmouse e1000e drm rtsx_pci wmi i2c_hid
pinctrl_sunrisepoint hid video pinctrl_intel [last unloaded: cifs]
[ 1432.735588] ---[ end trace 8a48b6964e24a1b1 ]---
[ 1432.735593] RIP: 0010:usercopy_abort+0x7a/0x7c
[ 1432.735595] Code: 0f 45 c6 51 48 89 f9 48 c7 c2 45 9d 50 8e 41 52
48 c7 c6 29 8a 4f 8e 48 c7 c7 10 9e 50 8e 48 0f 45 f2 48 89 c2 e8 0f
03 e6 ff <0f> 0b 4d 89 e0 31 c9 44 89 ea 31 f6 48 c7 c7 79 9d 50 8e e8
6e ff
[ 1432.735597] RSP: 0018:ffffa8df102bfe30 EFLAGS: 00010246
[ 1432.735600] RAX: 0000000000000064 RBX: ffff9dadcddccea0 RCX: 0000000000000000
[ 1432.735602] RDX: 0000000000000000 RSI: ffff9daddf796428 RDI: ffff9daddf796428
[ 1432.735604] RBP: ffffa8df102bfe48 R08: 00000000000b388d R09: 00000000000004ef
[ 1432.735606] R10: 0000000000000004 R11: ffffffff8ed933ed R12: 000000000000001e
[ 1432.735608] R13: 0000000000000001 R14: ffff9dadcddccebe R15: ffff9dad35a62240
[ 1432.735611] FS:  00007fbc0779b740(0000) GS:ffff9daddf780000(0000)
knlGS:0000000000000000
[ 1432.735613] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1432.735615] CR2: 000055d070dd9008 CR3: 000000072ca1e005 CR4: 00000000003606e0
[ 1432.735617] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1432.735619] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
On Wed, Oct 10, 2018 at 12:29 AM Ronnie Sahlberg <lsahlber@xxxxxxxxxx> wrote:
>
> When processing the mids for compounds we would only add credits based on
> the last successful mid in the compound which would leak credits and
> eventually triggering a re-connect.
>
> Fix this by splitting the mid processing part into two loops instead of one
> where the first loop just waits for all mids and then counts how many
> credits we were granted for the whole compound.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@xxxxxxxxxx>
> ---
>  fs/cifs/transport.c | 57 ++++++++++++++++++++++++++++++++++-------------------
>  1 file changed, 37 insertions(+), 20 deletions(-)
>
> diff --git a/fs/cifs/transport.c b/fs/cifs/transport.c
> index b48f43963da6..333729cf46cd 100644
> --- a/fs/cifs/transport.c
> +++ b/fs/cifs/transport.c
> @@ -786,7 +786,7 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>         int i, j, rc = 0;
>         int timeout, optype;
>         struct mid_q_entry *midQ[MAX_COMPOUND];
> -       unsigned int credits = 1;
> +       unsigned int credits = 0;
>         char *buf;
>
>         timeout = flags & CIFS_TIMEOUT_MASK;
> @@ -851,17 +851,20 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>
>         mutex_unlock(&ses->server->srv_mutex);
>
> -       for (i = 0; i < num_rqst; i++) {
> -               if (rc < 0)
> -                       goto out;
> +       if (rc < 0)
> +               goto out;
>
> -               if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP))
> -                       smb311_update_preauth_hash(ses, rqst[i].rq_iov,
> -                                                  rqst[i].rq_nvec);
> +       /*
> +        * Compounding is never used during session establish.
> +        */
> +       if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP))
> +               smb311_update_preauth_hash(ses, rqst[0].rq_iov,
> +                                          rqst[0].rq_nvec);
>
> -               if (timeout == CIFS_ASYNC_OP)
> -                       goto out;
> +       if (timeout == CIFS_ASYNC_OP)
> +               goto out;
>
> +       for (i = 0; i < num_rqst; i++) {
>                 rc = wait_for_response(ses->server, midQ[i]);
>                 if (rc != 0) {
>                         cifs_dbg(FYI, "Cancelling wait for mid %llu\n",
> @@ -877,10 +880,21 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>                         }
>                         spin_unlock(&GlobalMid_Lock);
>                 }
> +       }
> +
> +       for (i = 0; i < num_rqst; i++)
> +               if (midQ[i]->resp_buf)
> +                       credits += ses->server->ops->get_credits(midQ[i]);
> +       if (!credits)
> +               credits = 1;
> +
> +       for (i = 0; i < num_rqst; i++) {
> +               if (rc < 0)
> +                       goto out;
>
>                 rc = cifs_sync_mid_result(midQ[i], ses->server);
>                 if (rc != 0) {
> -                       add_credits(ses->server, 1, optype);
> +                       add_credits(ses->server, credits, optype);
>                         return rc;
>                 }
>
> @@ -901,23 +915,26 @@ compound_send_recv(const unsigned int xid, struct cifs_ses *ses,
>                 else
>                         resp_buf_type[i] = CIFS_SMALL_BUFFER;
>
> -               if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP)) {
> -                       struct kvec iov = {
> -                               .iov_base = resp_iov[i].iov_base,
> -                               .iov_len = resp_iov[i].iov_len
> -                       };
> -                       smb311_update_preauth_hash(ses, &iov, 1);
> -               }
> -
> -               credits = ses->server->ops->get_credits(midQ[i]);
> -
>                 rc = ses->server->ops->check_receive(midQ[i], ses->server,
>                                                      flags & CIFS_LOG_ERROR);
>
>                 /* mark it so buf will not be freed by cifs_delete_mid */
>                 if ((flags & CIFS_NO_RESP) == 0)
>                         midQ[i]->resp_buf = NULL;
> +
>         }
> +
> +       /*
> +        * Compounding is never used during session establish.
> +        */
> +       if ((ses->status == CifsNew) || (optype & CIFS_NEG_OP)) {
> +               struct kvec iov = {
> +                       .iov_base = resp_iov[0].iov_base,
> +                       .iov_len = resp_iov[0].iov_len
> +               };
> +               smb311_update_preauth_hash(ses, &iov, 1);
> +       }
> +
>  out:
>         /*
>          * This will dequeue all mids. After this it is important that the
> --
> 2.13.6
>


-- 
Thanks,

Steve



[Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux