Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1521754

Re: [PATCH 3.2 009/152] ext4: check for extents that wrap around

Show all headers | View raw

On 11/14/2016 01:14 AM, Ben Hutchings wrote:
> 3.2.84-rc1 review patch.  If anyone has any objections, please let me know.

Just a general comment on stable review workflow, really:

It might be more useful to send the diff-of-diffs with the upstream
commit so I can easily see if you had any conflicts when cherry-picking
this and how they were resolved.

That's generally much more interesting than just the plain patch, where
I can't really tell if there were any changes at all (or conversely,
much more boring in case there were no changes, and thus easier to
review).

If you could push this commit to git before sending the review, you
could also include a command that I can use to quickly do the
diff-of-diffs myself without having to download and apply the patch (or
look for it), e.g. something like (using the 3.12 stable commit vs
upstream):

"""
diff -yw \
   <(echo upstream; git log -p -W f70749c^..f70749c) \
   <(echo 3.2; git log -p -W 33234c6^..33234c6)
"""

At least that would make it a lot easier for me (and I suspect other
casual stable contributors) to glance at a stable review email and tell
if the backport is correct or not. It should be pretty easy to script on
your end(s) for the benefit of everybody.

Just my 2 cents. Thanks,


Vegard

> ------------------
>
> From: Vegard Nossum <vegard.nossum@oracle.com>
>
> commit f70749ca42943faa4d4dcce46dfdcaadb1d0c4b6 upstream.
>
> An extent with lblock = 4294967295 and len = 1 will pass the
> ext4_valid_extent() test:
>
> 	ext4_lblk_t last = lblock + len - 1;
>
> 	if (len == 0 || lblock > last)
> 		return 0;
>
> since last = 4294967295 + 1 - 1 = 4294967295. This would later trigger
> the BUG_ON(es->es_lblk + es->es_len < es->es_lblk) in ext4_es_end().
>
> We can simplify it by removing the - 1 altogether and changing the test
> to use lblock + len <= lblock, since now if len = 0, then lblock + 0 ==
> lblock and it fails, and if len > 0 then lblock + len > lblock in order
> to pass (i.e. it doesn't overflow).
>
> Fixes: 5946d0893 ("ext4: check for overlapping extents in ext4_valid_extent_entries()")
> Fixes: 2f974865f ("ext4: check for zero length extent explicitly")
> Cc: Eryu Guan <guaneryu@gmail.com>
> Signed-off-by: Phil Turnbull <phil.turnbull@oracle.com>
> Signed-off-by: Vegard Nossum <vegard.nossum@oracle.com>
> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
> Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
> ---
>  fs/ext4/extents.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -319,9 +319,13 @@ static int ext4_valid_extent(struct inod
>  	ext4_fsblk_t block = ext4_ext_pblock(ext);
>  	int len = ext4_ext_get_actual_len(ext);
>  	ext4_lblk_t lblock = le32_to_cpu(ext->ee_block);
> -	ext4_lblk_t last = lblock + len - 1;
>
> -	if (len == 0 || lblock > last)
> +	/*
> +	 * We allow neither:
> +	 *  - zero length
> +	 *  - overflow/wrap-around
> +	 */
> +	if (lblock + len <= lblock)
>  		return 0;
>  	return ext4_data_block_valid(EXT4_SB(inode->i_sb), block, len);
>  }
>

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH 3.2 000/152] 3.2.84-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 070/152] cdc-acm: fix wrong pipe type on rx interrupt  xfers Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 108/152] frv: fix clear_user() Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 088/152] ALSA: timer: fix NULL pointer dereference on  memory allocation failure Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 044/152] KEYS: 64-bit MIPS needs to use  compat_sys_keyctl for 32-bit userspace Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 009/152] ext4: check for extents that wrap around Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
    Re: [PATCH 3.2 009/152] ext4: check for extents that wrap around Vegard Nossum <vegard.nossum@oracle.com> - 2016-11-14 16:40 +0100
      Re: [PATCH 3.2 009/152] ext4: check for extents that wrap around Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 17:20 +0100
  [PATCH 3.2 101/152] asm-generic: make copy_from_user() zero the  destination properly Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 077/152] USB: serial: option: add WeTelecom WM-D200 Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 125/152] microblaze: fix __get_user() Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 010/152] ext4: don't call ext4_should_journal_data()  on the journal inode Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 041/152] l2tp: Correctly return -EBADF from  pppol2tp_getname. Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 089/152] ALSA: timer: fix NULL pointer dereference in  read()/ioctl() race Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 045/152] drm/radeon: fix firmware info version checks Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 053/152] dm flakey: error READ bios during the  down_interval Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 030/152] crypto: scatterwalk - Fix test in  scatterwalk_done Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 131/152] openrisc: fix the fix of copy_from_user() Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 066/152] arm: oabi compat: add missing access checks Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 112/152] mn10300: copy_from_user() should zero on  access_ok() failure... Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 037/152] nfs: don't create zero-length requests Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:10 +0100
  [PATCH 3.2 015/152] x86/quirks: Apply nvidia_bugs quirk only on  root bus Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 013/152] ALSA: ctl: Stop notification after disconnection Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 019/152] NFS: Don't drop CB requests with invalid  principals Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 007/152] usb: renesas_usbhs: protect the CFIFOSEL  setting in usbhsg_ep_enable() Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 021/152] Bluetooth: Add support of 13d3:3490 AR3012 device Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 003/152] sched/cputime: Fix prev steal time accouting  during CPU hotplug Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 029/152] Bluetooth: Fix l2cap_sock_setsockopt() with  optname BT_RCVMTU Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 008/152] Input: xpad - validate USB endpoint count  during probe Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 036/152] MIPS: RM7000: Double locking bug in  rm7k_tc_disable() Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 018/152] svc: Avoid garbage replies when pc_func()  returns rpc_drop_reply Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 006/152] usb: renesas_usbhs: fix NULL pointer  dereference in xfer_work() Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 035/152] tty/vt/keyboard: fix OOB access in  do_compute_shiftstate() Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  [PATCH 3.2 012/152] drm/radeon: Poll for both connect/disconnect  on analog connectors Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 04:20 +0100
  Re: [PATCH 3.2 000/152] 3.2.84-rc1 review Guenter Roeck <linux@roeck-us.net> - 2016-11-14 06:50 +0100
    Re: [PATCH 3.2 000/152] 3.2.84-rc1 review Ben Hutchings <ben@decadent.org.uk> - 2016-11-14 18:20 +0100

csiph-web