On 17 Apr 2019, at 22:17, Gleb Smirnoff wrote:
  Kristof,

On Wed, Apr 17, 2019 at 04:42:54PM +0000, Kristof Provost wrote:
K> Modified: head/sys/netpfil/pf/pf_ioctl.c
K> ============================================================================== K> --- head/sys/netpfil/pf/pf_ioctl.c Wed Apr 17 16:31:30 2019 (r346318) K> +++ head/sys/netpfil/pf/pf_ioctl.c Wed Apr 17 16:42:54 2019 (r346319)
K> @@ -3103,24 +3103,24 @@ DIOCCHANGEADDR_error:
K>                   break;
K>           }
K>
K> -         PF_RULES_WLOCK();
K> +         PF_RULES_RLOCK();
K>           n = pfr_table_count(&io->pfrio_table, io->pfrio_flags);
K>           io->pfrio_size = min(io->pfrio_size, n);
K> +         PF_RULES_RUNLOCK();
K>
K>           totlen = io->pfrio_size * sizeof(struct pfr_table);
K>           pfrts = mallocarray(io->pfrio_size, sizeof(struct pfr_table),
K>               M_TEMP, M_NOWAIT);
K>           if (pfrts == NULL) {
K>                   error = ENOMEM;
K> -                 PF_RULES_WUNLOCK();
K>                   break;
K>           }
K>           error = copyin(io->pfrio_buffer, pfrts, totlen);
K>           if (error) {
K>                   free(pfrts, M_TEMP);
K> -                 PF_RULES_WUNLOCK();
K>                   break;
K>           }
K> +         PF_RULES_WLOCK();
K>           error = pfr_set_tflags(pfrts, io->pfrio_size,
K>               io->pfrio_setflag, io->pfrio_clrflag, &io->pfrio_nchange,
K>               &io->pfrio_ndel, io->pfrio_flags | PFR_FLAG_USERIOCTL);

Couple comments:

1) Now we can malloc with M_WAITOK.

That’s a good point. I’ll see about changing that tomorrow.

2) Are we sure that table count won't change while we dropped the lock?

No, the table count can indeed change while we’re unlocked. It doesn’t really matter though. The initial count only serves to limit the memory allocation to something sane. pfr_set_tflags() still does appropriate checks. It’s always been possible for the table count to change between user space preparing its request and it being handled in the kernel, so that was always a possibility.

Regards,
Kristof
_______________________________________________
[email protected] mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "[email protected]"

Reply via email to