Re: BUG in copy_page_to_iter() when iscsi sets ENABLE_CLUSTERING
Note that independent of what we do in the Linux iSCSI initiator
this is a network DOS, so we'll have to fix it.
On Wed, Dec 05, 2018 at 12:09:40PM -0800, Lee Duncan wrote:
> I recently found what I believe is a bug, and I'd appreciate feedback
> on if that is correct, and if so how to proceed.
>
> BACKGROUND
>
> Recently Christoph Hellwig sent an email to driver maintainers for
> drivers that set ".use_clustering" to DISABLE_CLUSTERING in their SCSI
> Host templates, asking if the setting could be changed to
> ENABLE_CLUSTERING.
>
> As part of answering that question, I set ENABLE_CLUSTERING in
> drivers/scsi/iscsi_tcp.c and tested both the iscsi initiator and
> target.
>
> As a reminder, setting ENABLE_CLUSTERING means that adjacent bio-s can
> be merged. This can make IO faster, but it means that drivers must be
> able to deal with IOs that cross page boundaries, since bio merges can
> create such IOs.
>
> RESULTS
>
> The iscsi initiator code can handle ENABLE_CLUSTERING just fine, but
> the iscsi target code fails. It seems to assume that IOs do *NOT*
> cross a page boundary.
>
> The problem is in iscsi lib/iov_iter.c, in the functions
> copy_page_to_iter() and page_copy_sane() (see below for how to
> reproduce):
>
> >> static inline bool page_copy_sane(struct page *page, size_t offset, size_t
> >> n)
> >> {
> >> struct page *head = compound_head(page);
> >> size_t v = n + offset + page_address(page) - page_address(head);
> >>
> >> if (likely(n <= v && v <= (PAGE_SIZE << compound_order(head
> >> return true;
> >> WARN_ON(1);
> >> return false;
> >> }
> >>
> >> size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes,
> >> struct iov_iter *i)
> >> {
> >> if (unlikely(!page_copy_sane(page, offset, bytes)))
> >> return 0;
> >> if (i->type & (ITER_BVEC|ITER_KVEC)) {
> >> void *kaddr = kmap_atomic(page);
> >> size_t wanted = copy_to_iter(kaddr + offset, bytes, i);
> >> kunmap_atomic(kaddr);
> >> return wanted;
> >> } else if (unlikely(iov_iter_is_discard(i)))
> >> return bytes;
> >> else if (likely(!iov_iter_is_pipe(i)))
> >> return copy_page_to_iter_iovec(page, offset, bytes, i);
> >> else
> >> return copy_page_to_iter_pipe(page, offset, bytes, i);
> >> }
>
> Causing the following WARN_ON stack trace (repeatedly):
>
> >> ...
> >> [ 78.644559] WARNING: CPU: 0 PID: 2192 at lib/iov_iter.c:830
> >> copy_page_to_iter+0x1a6/0x2e0
> >> [ 78.644561] Modules linked in: iscsi_tcp(E) libiscsi_tcp(E) libiscsi(E)
> >> scsi_transport_iscsi(E) rfcomm(E) iscsi_target_mod(E) target_core_pscsi(E)
> >> target_core_file(E) target_core_iblock(E) target_core_user(E) uio(E)
> >> target_core_mod(E) configfs(E) af_packet(E) iscsi_ibft(E)
> >> iscsi_boot_sysfs(E) vmw_vsock_vmci_transport(E) vsock(E) bnep(E) fuse(E)
> >> crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) xfs(E)
> >> aesni_intel(E) snd_ens1371(E) aes_x86_64(E) snd_ac97_codec(E)
> >> crypto_simd(E) cryptd(E) ac97_bus(E) glue_helper(E) snd_rawmidi(E)
> >> vmw_balloon(E) snd_seq_device(E) snd_pcm(E) pcspkr(E) snd_timer(E) snd(E)
> >> uvcvideo(E) btusb(E) videobuf2_vmalloc(E) btrtl(E) videobuf2_memops(E)
> >> videobuf2_v4l2(E) btbcm(E) btintel(E) videodev(E) bluetooth(E)
> >> videobuf2_common(E) vmw_vmci(E) ecdh_generic(E) rfkill(E) soundcore(E)
> >> mptctl(E) gameport(E) joydev(E) i2c_piix4(E) e1000(E) ac(E) button(E)
> >> btrfs(E) libcrc32c(E) xor(E) raid6_pq(E) hid_generic(E) usbhid(E)
> >> sr_mod(E) cdrom(E) ata_generic(E)
> >> [ 78.644583] crc32c_intel(E) serio_raw(E) mptspi(E)
> >> scsi_transport_spi(E) mptscsih(E) ata_piix(E) uhci_hcd(E) ehci_pci(E)
> >> ehci_hcd(E) vmwgfx(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E)
> >> sysimgblt(E) fb_sys_fops(E) usbcore(E) ttm(E) mptbase(E) drm(E) sg(E)
> >> dm_multipath(E) dm_mod(E) scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E)
> >> [ 78.644593] CPU: 0 PID: 2192 Comm: iscsi_trx Tainted: GE
> >> 4.20.0-rc4-1-default+ #1 openSUSE Tumbleweed (unreleased)
> >> [ 78.644593] Hardware name: VMware, Inc. VMware Virtual Platform/440BX
> >> Desktop Reference Platform, BIOS 6.00 05/19/2017
> >> [ 78.644595] RIP: 0010:copy_page_to_iter+0x1a6/0x2e0
> >> [ 78.644596] Code: 0c 24 48 98 49 89 ce 48 89 c2 49 29 c6 48 29 ca 4d 01
> >> f4 48 01 d5 48 85 c0 0f 85 71 ff ff ff 48 85 ed 0f 84 68 ff ff ff eb b2
> >> <0f> 0b 45 31 ed eb 8d bf 01 00 00 00 e8 d9 1e c5 ff 65 4c 8b 34 25
> >> [ 78.644596] RSP: 0018:a04b41eefbe0 EFLAGS: 00010206
> >> [ 78.644597] RAX: f7acc40de180 RBX: a04b41eefd80 RCX:
> >> 0028a014
> >> [ 78.644598] RDX: 2000 RSI: 1000 RDI:
> >> f7acc000
> >> [ 78.644598] RBP: f7acc40de180 R08: 8f1703786000 R09:
> >>
BUG in copy_page_to_iter() when iscsi sets ENABLE_CLUSTERING
I recently found what I believe is a bug, and I'd appreciate feedback
on if that is correct, and if so how to proceed.
BACKGROUND
Recently Christoph Hellwig sent an email to driver maintainers for
drivers that set ".use_clustering" to DISABLE_CLUSTERING in their SCSI
Host templates, asking if the setting could be changed to
ENABLE_CLUSTERING.
As part of answering that question, I set ENABLE_CLUSTERING in
drivers/scsi/iscsi_tcp.c and tested both the iscsi initiator and
target.
As a reminder, setting ENABLE_CLUSTERING means that adjacent bio-s can
be merged. This can make IO faster, but it means that drivers must be
able to deal with IOs that cross page boundaries, since bio merges can
create such IOs.
RESULTS
The iscsi initiator code can handle ENABLE_CLUSTERING just fine, but
the iscsi target code fails. It seems to assume that IOs do *NOT*
cross a page boundary.
The problem is in iscsi lib/iov_iter.c, in the functions
copy_page_to_iter() and page_copy_sane() (see below for how to
reproduce):
>> static inline bool page_copy_sane(struct page *page, size_t offset, size_t n)
>> {
>> struct page *head = compound_head(page);
>> size_t v = n + offset + page_address(page) - page_address(head);
>>
>> if (likely(n <= v && v <= (PAGE_SIZE << compound_order(head
>> return true;
>> WARN_ON(1);
>> return false;
>> }
>>
>> size_t copy_page_to_iter(struct page *page, size_t offset, size_t bytes,
>> struct iov_iter *i)
>> {
>> if (unlikely(!page_copy_sane(page, offset, bytes)))
>> return 0;
>> if (i->type & (ITER_BVEC|ITER_KVEC)) {
>> void *kaddr = kmap_atomic(page);
>> size_t wanted = copy_to_iter(kaddr + offset, bytes, i);
>> kunmap_atomic(kaddr);
>> return wanted;
>> } else if (unlikely(iov_iter_is_discard(i)))
>> return bytes;
>> else if (likely(!iov_iter_is_pipe(i)))
>> return copy_page_to_iter_iovec(page, offset, bytes, i);
>> else
>> return copy_page_to_iter_pipe(page, offset, bytes, i);
>> }
Causing the following WARN_ON stack trace (repeatedly):
>> ...
>> [ 78.644559] WARNING: CPU: 0 PID: 2192 at lib/iov_iter.c:830
>> copy_page_to_iter+0x1a6/0x2e0
>> [ 78.644561] Modules linked in: iscsi_tcp(E) libiscsi_tcp(E) libiscsi(E)
>> scsi_transport_iscsi(E) rfcomm(E) iscsi_target_mod(E) target_core_pscsi(E)
>> target_core_file(E) target_core_iblock(E) target_core_user(E) uio(E)
>> target_core_mod(E) configfs(E) af_packet(E) iscsi_ibft(E)
>> iscsi_boot_sysfs(E) vmw_vsock_vmci_transport(E) vsock(E) bnep(E) fuse(E)
>> crct10dif_pclmul(E) crc32_pclmul(E) ghash_clmulni_intel(E) xfs(E)
>> aesni_intel(E) snd_ens1371(E) aes_x86_64(E) snd_ac97_codec(E) crypto_simd(E)
>> cryptd(E) ac97_bus(E) glue_helper(E) snd_rawmidi(E) vmw_balloon(E)
>> snd_seq_device(E) snd_pcm(E) pcspkr(E) snd_timer(E) snd(E) uvcvideo(E)
>> btusb(E) videobuf2_vmalloc(E) btrtl(E) videobuf2_memops(E) videobuf2_v4l2(E)
>> btbcm(E) btintel(E) videodev(E) bluetooth(E) videobuf2_common(E) vmw_vmci(E)
>> ecdh_generic(E) rfkill(E) soundcore(E) mptctl(E) gameport(E) joydev(E)
>> i2c_piix4(E) e1000(E) ac(E) button(E) btrfs(E) libcrc32c(E) xor(E)
>> raid6_pq(E) hid_generic(E) usbhid(E) sr_mod(E) cdrom(E) ata_generic(E)
>> [ 78.644583] crc32c_intel(E) serio_raw(E) mptspi(E) scsi_transport_spi(E)
>> mptscsih(E) ata_piix(E) uhci_hcd(E) ehci_pci(E) ehci_hcd(E) vmwgfx(E)
>> drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E)
>> usbcore(E) ttm(E) mptbase(E) drm(E) sg(E) dm_multipath(E) dm_mod(E)
>> scsi_dh_rdac(E) scsi_dh_emc(E) scsi_dh_alua(E)
>> [ 78.644593] CPU: 0 PID: 2192 Comm: iscsi_trx Tainted: GE
>> 4.20.0-rc4-1-default+ #1 openSUSE Tumbleweed (unreleased)
>> [ 78.644593] Hardware name: VMware, Inc. VMware Virtual Platform/440BX
>> Desktop Reference Platform, BIOS 6.00 05/19/2017
>> [ 78.644595] RIP: 0010:copy_page_to_iter+0x1a6/0x2e0
>> [ 78.644596] Code: 0c 24 48 98 49 89 ce 48 89 c2 49 29 c6 48 29 ca 4d 01
>> f4 48 01 d5 48 85 c0 0f 85 71 ff ff ff 48 85 ed 0f 84 68 ff ff ff eb b2 <0f>
>> 0b 45 31 ed eb 8d bf 01 00 00 00 e8 d9 1e c5 ff 65 4c 8b 34 25
>> [ 78.644596] RSP: 0018:a04b41eefbe0 EFLAGS: 00010206
>> [ 78.644597] RAX: f7acc40de180 RBX: a04b41eefd80 RCX:
>> 0028a014
>> [ 78.644598] RDX: 2000 RSI: 1000 RDI:
>> f7acc000
>> [ 78.644598] RBP: f7acc40de180 R08: 8f1703786000 R09:
>> 2000
>> [ 78.644599] R10: 02c0 R11: 8f16b3f31400 R12:
>>
>> [ 78.644600] R13: 2000 R14: 0008 R15:
>> 3800
>> [ 78.644601] FS: () GS:8f1739c0()
>> knlGS:
>> [ 78.644601] CS: 0010 DS: ES: CR0: 80050033
>> [ 78.644602] CR2:
