Re: CVS commit: src/sys/opencrypto

2017-04-18 Thread coypu
On Tue, Apr 18, 2017 at 05:05:06PM +, Maya Rashish wrote: > XXX surrounding code seems fishy > > @@ -759,7 +759,6 @@ swcr_compdec(struct cryptodesc *crd, con > if (result < crd->crd_len) { > adj = result - crd->crd_len; > if (outtype == CRYPTO_BUF_MBUF) { > -

Re: CVS commit: src/sys/opencrypto

2015-11-27 Thread Paul Goyette
On Fri, 27 Nov 2015, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sat Nov 28 03:40:43 UTC 2015 Modified Files: src/sys/opencrypto: crypto.c Log Message: fix the build Thanks, and sorry for the breakage.

Re: CVS commit: src/sys/opencrypto

2014-01-27 Thread Paul Goyette
Taylor R Campbell wrote: It seems to me that the crux of the problem is that devsw_detach doesn't fail if the device is still in use, because we do keep no books about which devsws are still in use, so there's nothing to stop you from unloading a device's module before you've given a its close

Re: CVS commit: src/sys/opencrypto

2014-01-26 Thread David Laight
On Sun, Jan 26, 2014 at 11:04:31AM +1100, matthew green wrote: phase one: disable auto-unload. Phase 1 has been done. for the whole kernel? No. Only for this specific module. right - my suggestion was that since this problem affects a large class of modules, until that

Re: CVS commit: src/sys/opencrypto

2014-01-26 Thread Taylor R Campbell
Date: Sun, 26 Jan 2014 11:36:32 + From: David Laight da...@l8s.co.uk It also sounds like manual unloads of drivers can happen when the device is open - and that is also bad. A manual unload probably isn't going to race with open or close though. Disallowing unload

re: CVS commit: src/sys/opencrypto

2014-01-25 Thread matthew green
Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the process/thread

re: CVS commit: src/sys/opencrypto

2014-01-25 Thread Paul Goyette
Phase 1 has been done. On Sat, 25 Jan 2014, matthew green wrote: Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count

re: CVS commit: src/sys/opencrypto

2014-01-25 Thread matthew green
phase one: disable auto-unload. Phase 1 has been done. for the whole kernel?

re: CVS commit: src/sys/opencrypto

2014-01-25 Thread Paul Goyette
On Sun, 26 Jan 2014, matthew green wrote: phase one: disable auto-unload. Phase 1 has been done. for the whole kernel? No. Only for this specific module. - | Paul Goyette | PGP Key fingerprint: | E-mail

re: CVS commit: src/sys/opencrypto

2014-01-25 Thread matthew green
phase one: disable auto-unload. Phase 1 has been done. for the whole kernel? No. Only for this specific module. right - my suggestion was that since this problem affects a large class of modules, until that is fixed, we should disable auto unload globally. .mrg.

Re: CVS commit: src/sys/opencrypto

2014-01-24 Thread Christos Zoulas
In article 7458.1390534...@splode.eterna.com.au, matthew green m...@eterna.com.au wrote: Log Message: Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is

Re: CVS commit: src/sys/opencrypto

2014-01-24 Thread Paul Goyette
Some modules get auto-loaded by the syscall mechanism. We already have a mechanism to determine if any lwp's are currently executing these syscalls, and if yes we prevent auto-unload. For device-driver modules, there is currently no equivalent mechanism. They get auto-loaded from code in

Re: CVS commit: src/sys/opencrypto

2014-01-24 Thread Taylor R Campbell
Date: Fri, 24 Jan 2014 17:35:41 + (UTC) From: chris...@astron.com (Christos Zoulas) In article 7458.1390534...@splode.eterna.com.au, matthew green m...@eterna.com.au wrote: Log Message: XXX Also note that there is still a small window where the ref-count can XXX

Re: CVS commit: src/sys/opencrypto

2014-01-24 Thread Paul Goyette
On Fri, 24 Jan 2014, Taylor R Campbell wrote: Shouldn't devsw_detach or config_fini_component handle this? Why does the crypto device need special reference counting that other devices don't? The crypto device isn't special in this regard. Pretty much all device driver modules need this

re: CVS commit: src/sys/opencrypto

2014-01-23 Thread matthew green
Log Message: Implement in-module ref-counting, and do not allow auto-unload if there are existing references. Note that manual unloading is not prevented. OK christos@ XXX Also note that there is still a small window where the ref-count can XXX be decremented, and then the

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Paul Goyette
On Sun, 19 Jan 2014, Christos Zoulas wrote: Module Name:src Committed By: christos Date: Sun Jan 19 18:16:13 UTC 2014 Modified Files: src/sys/opencrypto: cryptodev.c Log Message: bail out unloading for now How about the following changes? @@ -143,6 +143,8 @@ static

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Christos Zoulas
On Jan 19, 10:22am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | How about the following changes? You need to handle the regular open too, not justthe get (look for the other fd_clone) | @@ -143,6 +143,8 @@ static intcryptoread(dev_t dev

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Christos Zoulas
On Jan 19, 10:37am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | That's covered in cryptoopen() at line 1060 I missed that patch | Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() | which attempts to detach each device instance

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Paul Goyette
On Sun, 19 Jan 2014, Christos Zoulas wrote: On Jan 19, 10:37am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | That's covered in cryptoopen() at line 1060 I missed that patch No worry. | Handled indirectly. The MODULE_CMD_FINI calls

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Christos Zoulas
On Jan 19, 11:21am, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | | Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() | | which attempts to detach each device instance. If a detach fails, then | | config_cfdata_detach fails

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Paul Goyette
On Sun, 19 Jan 2014, John Nemeth wrote: } Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() } which attempts to detach each device instance. If a detach fails, then } config_cfdata_detach fails, and the unload will fail. Does this mean that you'll end up with some

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Christos Zoulas
In article pine.neb.4.64.1401191416060.22...@screamer.whooppee.com, Paul Goyette p...@whooppee.com wrote: On Sun, 19 Jan 2014, John Nemeth wrote: } Handled indirectly. The MODULE_CMD_FINI calls config_cfdata_detach() } which attempts to detach each device instance. If a detach fails, then }

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread John Nemeth
On Jan 19, 10:37am, Paul Goyette wrote: } On Sun, 19 Jan 2014, Christos Zoulas wrote: } On Jan 19, 10:22am, p...@whooppee.com (Paul Goyette) wrote: } } | How about the following changes? } } You need to handle the regular open too, not justthe get (look for the } other fd_clone) } } That's

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Paul Goyette
The mere existence of a non-zero unit is a reference that needs to prevent unloading. The two checks (unit# and ref-count) are equivalent and redundant, and only one of them needs to be there. On Sun, 19 Jan 2014, Christos Zoulas wrote: In article

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Christos Zoulas
On Jan 19, 3:04pm, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | The mere existence of a non-zero unit is a reference that needs to | prevent unloading. What if it is the last reference? Then the ref count will go to zero after close without unloading

Re: CVS commit: src/sys/opencrypto

2014-01-19 Thread Paul Goyette
On Sun, 19 Jan 2014, Christos Zoulas wrote: On Jan 19, 3:04pm, p...@whooppee.com (Paul Goyette) wrote: -- Subject: Re: CVS commit: src/sys/opencrypto | The mere existence of a non-zero unit is a reference that needs to | prevent unloading. What if it is the last reference? Then the ref count