Project

General

Profile

Bug #6314

buffer overflow in dsl_dataset_name

Added by Arne Jansen about 4 years ago. Updated over 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Matthew Ahrens
Category:
zfs - Zettabyte File System
Start date:
2015-10-09
Due date:
% Done:

100%

Estimated time:
Difficulty:
Bite-size
Tags:
needs-triage

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

0001-6314-buffer-overflow-in-dsl_dataset_name.patch (1.03 KB) 0001-6314-buffer-overflow-in-dsl_dataset_name.patch Git patch Simon Klinkert, 2015-10-15 08:15 AM

Related issues

Related to illumos gate - Bug #8713: Buffer overflow in dsl_dataset_name()Closed2017-10-11

Actions
Is duplicate of illumos gate - Bug #7014: panic buffer overflow in dsl_dataset_nameRejected2016-05-30

Actions

History

#1

Updated by Simon Klinkert about 4 years ago

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 ()
#2

Updated by Matthew Ahrens about 4 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.

#3

Updated by Matthew Ahrens about 4 years ago

  • Category set to zfs - Zettabyte File System
  • Assignee set to Matthew Ahrens

I am working on a fix for this.

#4

Updated by Matthew Ahrens over 3 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.

#5

Updated by Igor Kozhukhov over 3 years ago

  • Is duplicate of Bug #7014: panic buffer overflow in dsl_dataset_name added
#6

Updated by Electric Monk over 3 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>

#7

Updated by Ezomori Nozomu about 2 years ago

  • Related to Bug #8713: Buffer overflow in dsl_dataset_name() added

Also available in: Atom PDF