Bug #6314
closed
buffer overflow in dsl_dataset_name
100%
Description
Callers of dsl_dataset_name pass a buffer of size ZFS_MAXNAMELEN, but dsl_dataset_name copies the datasets' name PLUS the snapshot name to it, resulting in a max of 2 * ZFS_MAXNAMELEN + '@'.
Files
Related issues
Updated by Simon K over 6 years ago
- File 0001-6314-buffer-overflow-in-dsl_dataset_name.patch 0001-6314-buffer-overflow-in-dsl_dataset_name.patch added
One simple way to reproduce:
$ zfs create xxxxx/mgmt/jefhbuyerjlfknrejlfjrelfjreflerjflrefjrlefrjelfrjefljrelfrelfnrelfreflrefrlferfrefuyefbvryegfirefygreuyfgriefygirefgriefygreifyerifygi67r43r947r34436fdg43fyfriufyrifrfrif $ zfs snapshot -r xxxxx/mgmt@jefhbuyerjlfknrejlfjrelfjreflerjflrefjrlefrjelfrjefljrelfrelfnrelfreflref Write failed: Broken pipe
I've created a patch which adds two asserts to fix the security concerns (see attachment):
panic[cpu10]/thread=ffffff007be1fc40: assertion failed: strlen(name) + strlen(ds->ds_snapname) + 1 <= (256 - 1) (0x100 <= 0xff), file: ../../common/fs/zfs/dsl_dataset.c, line: 663 ffffff007be1f5c0 fffffffffba8b64d () ffffff007be1f600 zfs:dsl_dataset_name+175 () ffffff007be1f7c0 zfs:spa_history_log_internal_ds+64 () ffffff007be1f860 zfs:dsl_dataset_snapshot_sync_impl+3d7 () ffffff007be1f9d0 zfs:dsl_dataset_snapshot_sync+6f () ffffff007be1fa10 zfs:dsl_sync_task_sync+10a () ffffff007be1faa0 zfs:dsl_pool_sync+285 () ffffff007be1fb70 zfs:spa_sync+27e () ffffff007be1fc20 zfs:txg_sync_thread+21f () ffffff007be1fc30 unix:thread_start+8 ()
Updated by Matthew Ahrens over 6 years ago
The bug is that we allowed a name > ZFS_MAXNAMELEN to be created in the first place. There is code in place to prevent this, in dsl_dataset_snapshot_check, dsl_dataset_rename_snapshot_check_impl, and dsl_valid_rename.
However, there is an off-by-one error due to the difference between ZFS_MAXNAMELEN as defined in zfs_znode.h (as MAXNAMELEN - 1, i.e. 255), and MAXNAMELEN (256). The fact that ZFS_MAXNAMELEN is defined to be MAXNAMELEN (256) in libzfs.h compounds the confusion.
The problem is that dsl_dataset_snapshot_check checks (strlen(name) >= MAXNAMELEN), when it should be using ZFS_MAXNAMELEN. We should examine all uses of ZFS_MAXNAMELEN and NAMELEN in ZFS code and make sure they are doing the right thing.
Adding the VERIFY seems good for safety, but is not a fix for the actual bug.
Updated by Matthew Ahrens about 6 years ago
- Category set to zfs - Zettabyte File System
- Assignee set to Matthew Ahrens
I am working on a fix for this.
Updated by Matthew Ahrens over 5 years ago
Fix is to introduce a new macro (ZFS_MAX_DATASET_NAME_LEN) to indicate the max length of a dataset name (including terminating NUL byte), and change everything to use that.
Updated by Igor Kozhukhov over 5 years ago
- Is duplicate of Bug #7014: panic buffer overflow in dsl_dataset_name added
Updated by Electric Monk over 5 years ago
- Status changed from New to Closed
- % Done changed from 0 to 100
git commit 9adfa60d484ce2435f5af77cc99dcd4e692b6660
commit 9adfa60d484ce2435f5af77cc99dcd4e692b6660
Author: Matthew Ahrens <mahrens@delphix.com>
Date: 2016-06-09T19:55:53.000Z
6314 buffer overflow in dsl_dataset_name
Reviewed by: George Wilson <george.wilson@delphix.com>
Reviewed by: Prakash Surya <prakash.surya@delphix.com>
Reviewed by: Igor Kozhukhov <ikozhukhov@gmail.com>
Approved by: Dan McDonald <danmcd@omniti.com>
Updated by Ezomori Nozomu over 4 years ago
- Related to Bug #8713: Buffer overflow in dsl_dataset_name() added