Bug 210864 - ggatel(8) BIO_DELETE implementation is broken
Summary: ggatel(8) BIO_DELETE implementation is broken
Status: New
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Only Me
Assignee: freebsd-bugs mailing list
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-07-06 02:22 UTC by Julian Hsiao
Modified: 2016-07-06 02:22 UTC (History)
0 users

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Hsiao 2016-07-06 02:22:05 UTC
md(4) simulates BIO_DELETE by writing zeros, and I infer ggatel(8) is
trying to do the same based on the code structure similarity. However, the
latter's implementation is actually broken. If
g_gate_ioctl(G_GATE_CMD_START, ...) returns with a BIO_DELETE request, only
gctl_length and gctl_offset are updated, and gctl_data still points to the
original buffer. ggatel(8) then proceeds to service BIO_DELETE as if it
were BIO_WRITE, so it writes "garbage" to disk, and (hopefully) EFAULT if
gctl_length is greater than gctl_data's actual length.

PoC:

# cd /tmp
# clang -x c -o bug - <<'BUG'
#include <assert.h>

#include <fcntl.h>
#include <sys/disk.h>
#include <sys/ioctl.h>

int main()
{
    const int fd = open("/dev/ggate0", O_RDWR | O_CLOEXEC | O_DIRECT);
    assert(fd != -1);

    off_t ol[2] = { 40960, 4096 };
    assert(ioctl(fd, DIOCGDELETE, ol) != -1);

    return(0);
}
BUG
# truncate -s 10m test
# ggatel create -s 4096 test
# dd if=/dev/zero   bs=4096 count=1 | LC_ALL=C tr '\0' '\252' |
  dd of=/dev/ggate0 bs=4096
# ./bug
# hexdump -C /dev/ggate0
00000000  aa aa aa aa aa aa aa aa  aa aa aa aa aa aa aa aa  |................|
*
00001000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
0000a000  aa aa aa aa aa aa aa aa  aa aa aa aa aa aa aa aa  |................|
*
0000b000  00 00 00 00 00 00 00 00  00 00 00 00 00 00 00 00  |................|
*
00a00000

I'm not sure of the security implication of this bug, since to trigger the
bug directly requires elevated privileges. However, there may be clever
ways to trigger it by proxy.

IMHO, simulating BIO_DELETE by writing zeros is questionable, however not
everyone agrees[0]. That said, you can always use md(4) instead, so I
suggest ggatel(8) should just return EOPNOTSUPP for BIO_DELETE.

[0] http://thread.gmane.org/gmane.os.freebsd.devel.hackers/57688/focus=57689