Age | Commit message (Collapse) | Author |
|
[ Upstream commit 4d42ecda239cc13738d6fd84d098a32e67b368b9 ]
indx_read is called when we have some NTFS directory operations that
need more information from the index buffers. This adds a sanity check
to make sure the returned index buffer length is legit, or we may have
some out-of-bound memory accesses.
[ 560.897595] BUG: KASAN: slab-out-of-bounds in hdr_find_e.isra.0+0x10c/0x320
[ 560.898321] Read of size 2 at addr ffff888009497238 by task exp/245
[ 560.898760]
[ 560.899129] CPU: 0 PID: 245 Comm: exp Not tainted 6.0.0-rc6 #37
[ 560.899505] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS rel-1.14.0-0-g155821a1990b-prebuilt.qemu.org 04/01/2014
[ 560.900170] Call Trace:
[ 560.900407] <TASK>
[ 560.900732] dump_stack_lvl+0x49/0x63
[ 560.901108] print_report.cold+0xf5/0x689
[ 560.901395] ? hdr_find_e.isra.0+0x10c/0x320
[ 560.901716] kasan_report+0xa7/0x130
[ 560.901950] ? hdr_find_e.isra.0+0x10c/0x320
[ 560.902208] __asan_load2+0x68/0x90
[ 560.902427] hdr_find_e.isra.0+0x10c/0x320
[ 560.902846] ? cmp_uints+0xe0/0xe0
[ 560.903363] ? cmp_sdh+0x90/0x90
[ 560.903883] ? ntfs_bread_run+0x190/0x190
[ 560.904196] ? rwsem_down_read_slowpath+0x750/0x750
[ 560.904969] ? ntfs_fix_post_read+0xe0/0x130
[ 560.905259] ? __kasan_check_write+0x14/0x20
[ 560.905599] ? up_read+0x1a/0x90
[ 560.905853] ? indx_read+0x22c/0x380
[ 560.906096] indx_find+0x2ef/0x470
[ 560.906352] ? indx_find_buffer+0x2d0/0x2d0
[ 560.906692] ? __kasan_kmalloc+0x88/0xb0
[ 560.906977] dir_search_u+0x196/0x2f0
[ 560.907220] ? ntfs_nls_to_utf16+0x450/0x450
[ 560.907464] ? __kasan_check_write+0x14/0x20
[ 560.907747] ? mutex_lock+0x8f/0xe0
[ 560.907970] ? __mutex_lock_slowpath+0x20/0x20
[ 560.908214] ? kmem_cache_alloc+0x143/0x4b0
[ 560.908459] ntfs_lookup+0xe0/0x100
[ 560.908788] __lookup_slow+0x116/0x220
[ 560.909050] ? lookup_fast+0x1b0/0x1b0
[ 560.909309] ? lookup_fast+0x13f/0x1b0
[ 560.909601] walk_component+0x187/0x230
[ 560.909944] link_path_walk.part.0+0x3f0/0x660
[ 560.910285] ? handle_lookup_down+0x90/0x90
[ 560.910618] ? path_init+0x642/0x6e0
[ 560.911084] ? percpu_counter_add_batch+0x6e/0xf0
[ 560.912559] ? __alloc_file+0x114/0x170
[ 560.913008] path_openat+0x19c/0x1d10
[ 560.913419] ? getname_flags+0x73/0x2b0
[ 560.913815] ? kasan_save_stack+0x3a/0x50
[ 560.914125] ? kasan_save_stack+0x26/0x50
[ 560.914542] ? __kasan_slab_alloc+0x6d/0x90
[ 560.914924] ? kmem_cache_alloc+0x143/0x4b0
[ 560.915339] ? getname_flags+0x73/0x2b0
[ 560.915647] ? getname+0x12/0x20
[ 560.916114] ? __x64_sys_open+0x4c/0x60
[ 560.916460] ? path_lookupat.isra.0+0x230/0x230
[ 560.916867] ? __isolate_free_page+0x2e0/0x2e0
[ 560.917194] do_filp_open+0x15c/0x1f0
[ 560.917448] ? may_open_dev+0x60/0x60
[ 560.917696] ? expand_files+0xa4/0x3a0
[ 560.917923] ? __kasan_check_write+0x14/0x20
[ 560.918185] ? _raw_spin_lock+0x88/0xdb
[ 560.918409] ? _raw_spin_lock_irqsave+0x100/0x100
[ 560.918783] ? _find_next_bit+0x4a/0x130
[ 560.919026] ? _raw_spin_unlock+0x19/0x40
[ 560.919276] ? alloc_fd+0x14b/0x2d0
[ 560.919635] do_sys_openat2+0x32a/0x4b0
[ 560.920035] ? file_open_root+0x230/0x230
[ 560.920336] ? __rcu_read_unlock+0x5b/0x280
[ 560.920813] do_sys_open+0x99/0xf0
[ 560.921208] ? filp_open+0x60/0x60
[ 560.921482] ? exit_to_user_mode_prepare+0x49/0x180
[ 560.921867] __x64_sys_open+0x4c/0x60
[ 560.922128] do_syscall_64+0x3b/0x90
[ 560.922369] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 560.923030] RIP: 0033:0x7f7dff2e4469
[ 560.923681] 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 088
[ 560.924451] RSP: 002b:00007ffd41a210b8 EFLAGS: 00000206 ORIG_RAX: 0000000000000002
[ 560.925168] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f7dff2e4469
[ 560.925655] RDX: 0000000000000000 RSI: 0000000000000002 RDI: 00007ffd41a211f0
[ 560.926085] RBP: 00007ffd41a252a0 R08: 00007f7dff60fba0 R09: 00007ffd41a25388
[ 560.926405] R10: 0000000000400b80 R11: 0000000000000206 R12: 00000000004004e0
[ 560.926867] R13: 00007ffd41a25380 R14: 0000000000000000 R15: 0000000000000000
[ 560.927241] </TASK>
[ 560.927491]
[ 560.927755] Allocated by task 245:
[ 560.928409] kasan_save_stack+0x26/0x50
[ 560.929271] __kasan_kmalloc+0x88/0xb0
[ 560.929778] __kmalloc+0x192/0x320
[ 560.930023] indx_read+0x249/0x380
[ 560.930224] indx_find+0x2a2/0x470
[ 560.930695] dir_search_u+0x196/0x2f0
[ 560.930892] ntfs_lookup+0xe0/0x100
[ 560.931115] __lookup_slow+0x116/0x220
[ 560.931323] walk_component+0x187/0x230
[ 560.931570] link_path_walk.part.0+0x3f0/0x660
[ 560.931791] path_openat+0x19c/0x1d10
[ 560.932008] do_filp_open+0x15c/0x1f0
[ 560.932226] do_sys_openat2+0x32a/0x4b0
[ 560.932413] do_sys_open+0x99/0xf0
[ 560.932709] __x64_sys_open+0x4c/0x60
[ 560.933417] do_syscall_64+0x3b/0x90
[ 560.933776] entry_SYSCALL_64_after_hwframe+0x63/0xcd
[ 560.934235]
[ 560.934486] The buggy address belongs to the object at ffff888009497000
[ 560.934486] which belongs to the cache kmalloc-512 of size 512
[ 560.935239] The buggy address is located 56 bytes to the right of
[ 560.935239] 512-byte region [ffff888009497000, ffff888009497200)
[ 560.936153]
[ 560.937326] The buggy address belongs to the physical page:
[ 560.938228] page:0000000062a3dfae refcount:1 mapcount:0 mapping:0000000000000000 index:0x0 pfn:0x9496
[ 560.939616] head:0000000062a3dfae order:1 compound_mapcount:0 compound_pincount:0
[ 560.940219] flags: 0xfffffc0010200(slab|head|node=0|zone=1|lastcpupid=0x1fffff)
[ 560.942702] raw: 000fffffc0010200 ffffea0000164f80 dead000000000005 ffff888001041c80
[ 560.943932] raw: 0000000000000000 0000000080080008 00000001ffffffff 0000000000000000
[ 560.944568] page dumped because: kasan: bad access detected
[ 560.945735]
[ 560.946112] Memory state around the buggy address:
[ 560.946870] ffff888009497100: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 560.947242] ffff888009497180: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
[ 560.947611] >ffff888009497200: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 560.947915] ^
[ 560.948249] ffff888009497280: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
[ 560.948687] ffff888009497300: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
Signed-off-by: Edward Lo <edward.lo@ambergroup.io>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
|
|
Fixes xfstest generic/300
Fixes: 4534a70b7056 ("fs/ntfs3: Add headers and misc files")
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
There is no need to initialize with NULL as it'll be rewritten later.
Signed-off-by: Li kunyu <kunyu@nfschina.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
This value is checked in indx_read, so it must be initialized
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Yan Lei <chinayanlei2002@163.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
This commit makes function a bit more readable
Cc: Joe Perches <joe@perches.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
'fnd' has been dereferenced several time before, so testing it here is
pointless.
Moreover, all callers of 'indx_find()' already have some error handling
code that makes sure that no NULL 'fnd' is passed.
So, remove the useless test.
Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
Reviewed-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
We do not have any reason to keep old linear search in. Before this was
used for error path or if table was so big that it cannot be allocated.
Current binary search implementation won't need error path. Remove old
references to linear entry search.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
We could try to optimize algorithm to first fill just small table and
after that use bigger table all the way up to ARRAY_SIZE(offs). This
way we can use bigger search array, but not lose benefits with entry
count smaller < ARRAY_SIZE(offs).
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
Current binary search allocates memory for table and fill whole table
before we start actual binary search. This is quite inefficient because
table fill will always be O(n). Also if table is huge we need to
reallocate memory which is costly.
This implementation use just stack memory and always when table is full
we will check if last element is <= and if not start table fill again.
The idea was that it would be same cost as table reallocation.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
We have lot of unnecessary headers in these files. Remove them so that
we help compiler a little bit.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
The variable err is being initialized with a value that is never read, it
is being updated later on. The assignment is redundant and can be removed.
Addresses-Coverity: ("Unused value")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Reviewed-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
Rename now works "Add new name and remove old name".
"Remove old name and add new name" may result in bad inode
if we can't add new name and then can't restore (add) old name.
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
Capitalize comments and end with period for better reading.
Also function comments are now little more kernel-doc style. This way we
can easily convert them to kernel-doc style if we want. Note that these
are not yet complete with this style. Example function comments start
with /* and in kernel-doc style they start /**.
Use imperative mood in function descriptions.
Change words like ntfs -> NTFS, linux -> Linux.
Use "we" not "I" when commenting code.
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
There are three bugs in this code:
1) If indx_get_root() fails, then return -EINVAL instead of success.
2) On the "/* make root external */" -EOPNOTSUPP; error path it should
free "re" but it has a memory leak.
3) If indx_new() fails then it will lead to an error pointer dereference
when we call put_indx_node().
I've re-written the error handling to be more clear.
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
The "e" pointer is dereferenced before it has been checked for NULL.
Move the dereference after the NULL check to prevent an Oops.
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
Return -EINVAL if ni_find_attr() fails. Don't return success.
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
Reviewed-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
Problem with these wrapper is that we cannot take off example GFP_NOFS
flag. It is not recomended use those in all places. Also if we change
one driver specific wrapper to kernel wrapper then it would look really
weird. People should be most familiar with kernel wrappers so let's just
use those ones.
Driver specific alloc wrapper also confuse some static analyzing tools,
good example is example kernels checkpatch tool. After we converter
these to kernel specific then warnings is showed.
Following Coccinelle script was used to automate changing.
virtual patch
@alloc depends on patch@
expression x;
expression y;
@@
(
- ntfs_malloc(x)
+ kmalloc(x, GFP_NOFS)
|
- ntfs_zalloc(x)
+ kzalloc(x, GFP_NOFS)
|
- ntfs_vmalloc(x)
+ kvmalloc(x, GFP_NOFS)
|
- ntfs_free(x)
+ kfree(x)
|
- ntfs_vfree(x)
+ kvfree(x)
|
- ntfs_memdup(x, y)
+ kmemdup(x, y, GFP_NOFS)
)
Reviewed-by: Christoph Hellwig <hch@lst.de>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
The static checkers (Smatch) were complaining because QuadAlign() was
buggy. If you try to align something higher than UINT_MAX it got
truncated to a u32.
Smatch warning was:
fs/ntfs3/attrib.c:383 attr_set_size_res()
warn: was expecting a 64 bit value instead of '~7'
So that this will not happen again we will change all these macros to
kernel made ones. This can also help some other static analyzing tools
to give us better warnings.
Patch was generated with Coccinelle script and after that some style
issue was hand fixed.
Coccinelle script:
virtual patch
@alloc depends on patch@
expression x;
@@
(
- #define QuadAlign(n) (((n) + 7u) & (~7u))
|
- QuadAlign(x)
+ ALIGN(x, 8)
|
- #define IsQuadAligned(n) (!((size_t)(n)&7u))
|
- IsQuadAligned(x)
+ IS_ALIGNED(x, 8)
|
- #define Quad2Align(n) (((n) + 15u) & (~15u))
|
- Quad2Align(x)
+ ALIGN(x, 16)
|
- #define IsQuad2Aligned(n) (!((size_t)(n)&15u))
|
- IsQuad2Aligned(x)
+ IS_ALIGNED(x, 16)
|
- #define Quad4Align(n) (((n) + 31u) & (~31u))
|
- Quad4Align(x)
+ ALIGN(x, 32)
|
- #define IsSizeTAligned(n) (!((size_t)(n) & (sizeof(size_t) - 1)))
|
- IsSizeTAligned(x)
+ IS_ALIGNED(x, sizeof(size_t))
|
- #define DwordAlign(n) (((n) + 3u) & (~3u))
|
- DwordAlign(x)
+ ALIGN(x, 4)
|
- #define IsDwordAligned(n) (!((size_t)(n)&3u))
|
- IsDwordAligned(x)
+ IS_ALIGNED(x, 4)
|
- #define WordAlign(n) (((n) + 1u) & (~1u))
|
- WordAlign(x)
+ ALIGN(x, 2)
|
- #define IsWordAligned(n) (!((size_t)(n)&1u))
|
- IsWordAligned(x)
+ IS_ALIGNED(x, 2)
|
)
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
Signed-off-by: Kari Argillander <kari.argillander@gmail.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
Fix the following fallthrough warnings:
fs/ntfs3/inode.c:1792:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
fs/ntfs3/index.c:178:2: warning: unannotated fall-through between switch labels [-Wimplicit-fallthrough]
This helps with the ongoing efforts to globally enable
-Wimplicit-fallthrough for Clang.
Link: https://github.com/KSPP/linux/issues/115
Signed-off-by: Gustavo A. R. Silva <gustavoars@kernel.org>
Reviewed-by: Nathan Chancellor <nathan@kernel.org>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
The multiplication of the u32 data_size with a int is being performed
using 32 bit arithmetic however the results is being assigned to the
variable nbits that is a size_t (64 bit) value. Fix a potential
integer overflow by casting the u32 value to a size_t before the
multiply to use a size_t sized bit multiply operation.
Addresses-Coverity: ("Unintentional integer overflow")
Fixes: 82cae269cfa9 ("fs/ntfs3: Add initialization of super block")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|
|
This adds initialization of super block
Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com>
|