Hello,
On Tue, Feb 14, 2023 at 01:23:16AM +0100, Alexander Bluhm wrote:
> On Mon, Feb 13, 2023 at 08:39:39AM +0100, Alexandr Nedvedicky wrote:
> > this bug has been found and reported by Hrvoje@ [1].
> > I took my chance and asked Hrvoje to test a small diff [2].
> > I would like to ask for OK to commit this fix which makes
> > Hrvoje's test box happy. Diff below is same to one found
> > at bugs@. The thing is that pfsync_bulk_update() function
> > must check first if there is anything to update, otherwise
> > we may day due to NULL pointer dereference.
>
> Your check makes sense, OK bluhm@
>
> But who is protecting write access to sc->sc_bulk_next ?
>
> I think it is exclusive netlock. This works as pfsync_input() is
> a protocol input function which is still not running in parallel.
yes, NET_LOCK() is still exclusive lock. so it should provide
sufficient protection.
>
> rw_enter_read(&pf_state_list.pfs_rwl) does not protect sc->sc_bulk_next
> it is a read lock. mtx_enter(&pf_state_list.pfs_mtx) is not taken
> for all writers to sc->sc_bulk_next.
>
> Do you have plans to relax this code from exclusive to shared
> netlock?
dlg@ has a new code for pfsync. I've seen his code back in ?December?
last year. support for bulk transfers was missing piece.
I think option we have here in pfsync(4) is to introduce a new rw-lock
private to if_pfsync and remove NET_LOCK() here completely.
sashan
