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
_______________________________________________
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to