[dpdk-stable] [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd

[dpdk-stable] [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd

[wangyunjian]

From: Yunjian Wang <wangyunjian@huawei.com>

Zero is a valid fd. It will fail to check the fd if the fd is zero.
The "job_ring->uio_fd" is an fd, so define it as "int".

Fixes: e7a45f3cc245 ("crypto/caam_jr: add UIO specific operations")
Fixes: a5e1018d5e67 ("crypto/caam_jr: add routines to configure HW")
Cc: stable@dpdk.org

Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
---
v2:
 * Change "job_ring->uio_fd" type suggested by Gagandeep Singh
---
 drivers/crypto/caam_jr/caam_jr.c             |  6 ++++--
 drivers/crypto/caam_jr/caam_jr_hw_specific.h |  2 +-
 drivers/crypto/caam_jr/caam_jr_pvt.h         |  5 +++--
 drivers/crypto/caam_jr/caam_jr_uio.c         | 21 ++++++++++++++------
 4 files changed, 23 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/caam_jr/caam_jr.c b/drivers/crypto/caam_jr/caam_jr.c
index 5a29dd169..f62d46546 100644
--- a/drivers/crypto/caam_jr/caam_jr.c
+++ b/drivers/crypto/caam_jr/caam_jr.c
@@ -2074,7 +2074,7 @@ static struct rte_security_ops caam_jr_security_ops = {
 static void
 close_job_ring(struct sec_job_ring_t *job_ring)
 {
-	if (job_ring->irq_fd) {
+	if (job_ring->irq_fd != -1) {
 		/* Producer index is frozen. If consumer index is not equal
 		 * with producer index, then we have descs to flush.
 		 */
@@ -2187,7 +2187,7 @@ caam_jr_dev_uninit(struct rte_cryptodev *dev)
  *
  */
 static void *
-init_job_ring(void *reg_base_addr, uint32_t irq_id)
+init_job_ring(void *reg_base_addr, int irq_id)
 {
 	struct sec_job_ring_t *job_ring = NULL;
 	int i, ret = 0;
@@ -2466,6 +2466,8 @@ RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv, cryptodev_caam_jr_drv.driver,
 
 RTE_INIT(caam_jr_init_log)
 {
+	sec_init();
+
 	caam_jr_logtype = rte_log_register("pmd.crypto.caam");
 	if (caam_jr_logtype >= 0)
 		rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE);
diff --git a/drivers/crypto/caam_jr/caam_jr_hw_specific.h b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
index 5f58a585d..ec4539d1b 100644
--- a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
+++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
@@ -360,7 +360,7 @@ struct sec_job_ring_t {
 				 * bitwise operations.
 				 */
 
-	uint32_t irq_fd;	/* The file descriptor used for polling from
+	int irq_fd;	/* The file descriptor used for polling from
 				 * user space for interrupts notifications
 				 */
 	uint32_t jr_mode;	/* Model used by SEC Driver to receive
diff --git a/drivers/crypto/caam_jr/caam_jr_pvt.h b/drivers/crypto/caam_jr/caam_jr_pvt.h
index 98cd4438a..fa9e86ea6 100644
--- a/drivers/crypto/caam_jr/caam_jr_pvt.h
+++ b/drivers/crypto/caam_jr/caam_jr_pvt.h
@@ -216,16 +216,17 @@ calc_chksum(void *buffer, int len)
 }
 struct uio_job_ring {
 	uint32_t jr_id;
-	uint32_t uio_fd;
+	int uio_fd;
 	void *register_base_addr;
 	int map_size;
 	int uio_minor_number;
 };
 
 int sec_cleanup(void);
+void sec_init(void);
 int sec_configure(void);
 struct uio_job_ring *config_job_ring(void);
-void free_job_ring(uint32_t uio_fd);
+void free_job_ring(int uio_fd);
 
 /* For Dma memory allocation of specified length and alignment */
 static inline void *
diff --git a/drivers/crypto/caam_jr/caam_jr_uio.c b/drivers/crypto/caam_jr/caam_jr_uio.c
index b1bb44ca4..133b32084 100644
--- a/drivers/crypto/caam_jr/caam_jr_uio.c
+++ b/drivers/crypto/caam_jr/caam_jr_uio.c
@@ -145,7 +145,7 @@ file_read_first_line(const char root[], const char subdir[],
 		 "%s/%s/%s", root, subdir, filename);
 
 	fd = open(absolute_file_name, O_RDONLY);
-	SEC_ASSERT(fd > 0, fd, "Error opening file %s",
+	SEC_ASSERT(fd >= 0, fd, "Error opening file %s",
 			absolute_file_name);
 
 	/* read UIO device name from first line in file */
@@ -322,7 +322,7 @@ uio_map_registers(int uio_device_fd, int uio_device_id,
 }
 
 void
-free_job_ring(uint32_t uio_fd)
+free_job_ring(int uio_fd)
 {
 	struct uio_job_ring *job_ring = NULL;
 	int i;
@@ -347,7 +347,7 @@ free_job_ring(uint32_t uio_fd)
 			job_ring->jr_id, job_ring->uio_fd);
 	close(job_ring->uio_fd);
 	g_uio_jr_num--;
-	job_ring->uio_fd = 0;
+	job_ring->uio_fd = -1;
 	if (job_ring->register_base_addr == NULL)
 		return;
 
@@ -370,7 +370,7 @@ uio_job_ring *config_job_ring(void)
 	int i;
 
 	for (i = 0; i < MAX_SEC_JOB_RINGS; i++) {
-		if (g_uio_job_ring[i].uio_fd == 0) {
+		if (g_uio_job_ring[i].uio_fd == -1) {
 			job_ring = &g_uio_job_ring[i];
 			g_uio_jr_num++;
 			break;
@@ -389,7 +389,7 @@ uio_job_ring *config_job_ring(void)
 
 	/* Open device file */
 	job_ring->uio_fd = open(uio_device_file_name, O_RDWR);
-	SEC_ASSERT(job_ring->uio_fd > 0, NULL,
+	SEC_ASSERT(job_ring->uio_fd >= 0, NULL,
 		"Failed to open UIO device file for job ring %d",
 		job_ring->jr_id);
 
@@ -488,12 +488,21 @@ sec_cleanup(void)
 		/* I need to close the fd after shutdown UIO commands need to be
 		 * sent using the fd
 		 */
-		if (job_ring->uio_fd != 0) {
+		if (job_ring->uio_fd != -1) {
 			CAAM_JR_INFO(
 			"Closed device file for job ring %d , fd = %d",
 			job_ring->jr_id, job_ring->uio_fd);
 			close(job_ring->uio_fd);
+			job_ring->uio_fd = -1;
 		}
 	}
 	return 0;
 }
+
+void sec_init(void)
+{
+	int i;
+
+	for (i = 0; i < g_uio_jr_num; i++)
+		g_uio_job_ring[i].uio_fd = -1;
+}
-- 
2.19.1

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd

[Gagandeep Singh]

> -----Original Message-----
> From: wangyunjian <wangyunjian@huawei.com>
> Sent: Monday, May 11, 2020 10:07 AM
> To: dev@dpdk.org
> Cc: Gagandeep Singh <G.Singh@nxp.com>; Hemant Agrawal
> <hemant.agrawal@nxp.com>; jerry.lilijun@huawei.com;
> xudingke@huawei.com; Yunjian Wang <wangyunjian@huawei.com>;
> stable@dpdk.org
> Subject: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd
> 
> From: Yunjian Wang <wangyunjian@huawei.com>
> 
> Zero is a valid fd. It will fail to check the fd if the fd is zero.
> The "job_ring->uio_fd" is an fd, so define it as "int".
> 
> Fixes: e7a45f3cc245 ("crypto/caam_jr: add UIO specific operations")
> Fixes: a5e1018d5e67 ("crypto/caam_jr: add routines to configure HW")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> ---
> v2:
>  * Change "job_ring->uio_fd" type suggested by Gagandeep Singh
> ---
>  drivers/crypto/caam_jr/caam_jr.c             |  6 ++++--
>  drivers/crypto/caam_jr/caam_jr_hw_specific.h |  2 +-
>  drivers/crypto/caam_jr/caam_jr_pvt.h         |  5 +++--
>  drivers/crypto/caam_jr/caam_jr_uio.c         | 21 ++++++++++++++------
>  4 files changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/crypto/caam_jr/caam_jr.c
> b/drivers/crypto/caam_jr/caam_jr.c
> index 5a29dd169..f62d46546 100644
> --- a/drivers/crypto/caam_jr/caam_jr.c
> +++ b/drivers/crypto/caam_jr/caam_jr.c
> @@ -2074,7 +2074,7 @@ static struct rte_security_ops caam_jr_security_ops =
> {
>  static void
>  close_job_ring(struct sec_job_ring_t *job_ring)
>  {
> -	if (job_ring->irq_fd) {
> +	if (job_ring->irq_fd != -1) {
>  		/* Producer index is frozen. If consumer index is not equal
>  		 * with producer index, then we have descs to flush.
>  		 */
> @@ -2187,7 +2187,7 @@ caam_jr_dev_uninit(struct rte_cryptodev *dev)
>   *
>   */
>  static void *
> -init_job_ring(void *reg_base_addr, uint32_t irq_id)
> +init_job_ring(void *reg_base_addr, int irq_id)
>  {
>  	struct sec_job_ring_t *job_ring = NULL;
>  	int i, ret = 0;
> @@ -2466,6 +2466,8 @@
> RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv,
> cryptodev_caam_jr_drv.driver,
> 
>  RTE_INIT(caam_jr_init_log)
>  {
> +	sec_init();
> +
Why are you calling a fd initialization function in log registration constructor?
This is not the right place for any initialization other than logs.
Also,  sec_init function will not work here, as you are using "g_uio_jr_num" whose value will always be 0 during this function call.

>  	caam_jr_logtype = rte_log_register("pmd.crypto.caam");
>  	if (caam_jr_logtype >= 0)
>  		rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE);
> diff --git a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> index 5f58a585d..ec4539d1b 100644
> --- a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> +++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> @@ -360,7 +360,7 @@ struct sec_job_ring_t {
>  				 * bitwise operations.
>  				 */
> 
> -	uint32_t irq_fd;	/* The file descriptor used for polling from
> +	int irq_fd;	/* The file descriptor used for polling from
>  				 * user space for interrupts notifications
>  				 */
>  	uint32_t jr_mode;	/* Model used by SEC Driver to receive
> diff --git a/drivers/crypto/caam_jr/caam_jr_pvt.h
> b/drivers/crypto/caam_jr/caam_jr_pvt.h
> index 98cd4438a..fa9e86ea6 100644
> --- a/drivers/crypto/caam_jr/caam_jr_pvt.h
> +++ b/drivers/crypto/caam_jr/caam_jr_pvt.h
> @@ -216,16 +216,17 @@ calc_chksum(void *buffer, int len)
>  }
>  struct uio_job_ring {
>  	uint32_t jr_id;
> -	uint32_t uio_fd;
> +	int uio_fd;
>  	void *register_base_addr;
>  	int map_size;
>  	int uio_minor_number;
>  };
> 
>  int sec_cleanup(void);
> +void sec_init(void);
>  int sec_configure(void);
>  struct uio_job_ring *config_job_ring(void);
> -void free_job_ring(uint32_t uio_fd);
> +void free_job_ring(int uio_fd);
> 
>  /* For Dma memory allocation of specified length and alignment */
>  static inline void *
> diff --git a/drivers/crypto/caam_jr/caam_jr_uio.c
> b/drivers/crypto/caam_jr/caam_jr_uio.c
> index b1bb44ca4..133b32084 100644
> --- a/drivers/crypto/caam_jr/caam_jr_uio.c
> +++ b/drivers/crypto/caam_jr/caam_jr_uio.c
> @@ -145,7 +145,7 @@ file_read_first_line(const char root[], const char
> subdir[],
>  		 "%s/%s/%s", root, subdir, filename);
> 
>  	fd = open(absolute_file_name, O_RDONLY);
> -	SEC_ASSERT(fd > 0, fd, "Error opening file %s",
> +	SEC_ASSERT(fd >= 0, fd, "Error opening file %s",
>  			absolute_file_name);
> 
>  	/* read UIO device name from first line in file */
> @@ -322,7 +322,7 @@ uio_map_registers(int uio_device_fd, int uio_device_id,
>  }
> 
>  void
> -free_job_ring(uint32_t uio_fd)
> +free_job_ring(int uio_fd)
>  {
>  	struct uio_job_ring *job_ring = NULL;
>  	int i;

Please update the check  "if (!uio_fd)".

> @@ -347,7 +347,7 @@ free_job_ring(uint32_t uio_fd)
>  			job_ring->jr_id, job_ring->uio_fd);
>  	close(job_ring->uio_fd);
>  	g_uio_jr_num--;
> -	job_ring->uio_fd = 0;
> +	job_ring->uio_fd = -1;
>  	if (job_ring->register_base_addr == NULL)
>  		return;
> 
> @@ -370,7 +370,7 @@ uio_job_ring *config_job_ring(void)
>  	int i;
> 
>  	for (i = 0; i < MAX_SEC_JOB_RINGS; i++) {
> -		if (g_uio_job_ring[i].uio_fd == 0) {
> +		if (g_uio_job_ring[i].uio_fd == -1) {
>  			job_ring = &g_uio_job_ring[i];
>  			g_uio_jr_num++;
>  			break;
> @@ -389,7 +389,7 @@ uio_job_ring *config_job_ring(void)
> 
>  	/* Open device file */
>  	job_ring->uio_fd = open(uio_device_file_name, O_RDWR);
> -	SEC_ASSERT(job_ring->uio_fd > 0, NULL,
> +	SEC_ASSERT(job_ring->uio_fd >= 0, NULL,
>  		"Failed to open UIO device file for job ring %d",
>  		job_ring->jr_id);
> 
> @@ -488,12 +488,21 @@ sec_cleanup(void)
>  		/* I need to close the fd after shutdown UIO commands need to
> be
>  		 * sent using the fd
>  		 */
> -		if (job_ring->uio_fd != 0) {
> +		if (job_ring->uio_fd != -1) {
>  			CAAM_JR_INFO(
>  			"Closed device file for job ring %d , fd = %d",
>  			job_ring->jr_id, job_ring->uio_fd);
>  			close(job_ring->uio_fd);
> +			job_ring->uio_fd = -1;
>  		}
>  	}
>  	return 0;
>  }
> +
> +void sec_init(void)
> +{
> +	int i;
> +
> +	for (i = 0; i < g_uio_jr_num; i++)
> +		g_uio_job_ring[i].uio_fd = -1;
> +}
> --
> 2.19.1
> 

There are still some functions(sec_uio_send_command, caam_jr_enable_irqs, caam_jr_disable_irqs) in caam_jr_uio.c, which are accepting uio_fd parameter as uint32_t, please update them also.

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd

[wangyunjian]

> -----Original Message-----
> From: Gagandeep Singh [mailto:G.Singh@nxp.com]
> Sent: Tuesday, May 12, 2020 11:49 AM
> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Lilijun (Jerry)
> <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd
> 
> 
> 
> > -----Original Message-----
> > From: wangyunjian <wangyunjian@huawei.com>
> > Sent: Monday, May 11, 2020 10:07 AM
> > To: dev@dpdk.org
> > Cc: Gagandeep Singh <G.Singh@nxp.com>; Hemant Agrawal
> > <hemant.agrawal@nxp.com>; jerry.lilijun@huawei.com;
> > xudingke@huawei.com; Yunjian Wang <wangyunjian@huawei.com>;
> > stable@dpdk.org
> > Subject: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd
> >
> > From: Yunjian Wang <wangyunjian@huawei.com>
> >
> > Zero is a valid fd. It will fail to check the fd if the fd is zero.
> > The "job_ring->uio_fd" is an fd, so define it as "int".
> >
> > Fixes: e7a45f3cc245 ("crypto/caam_jr: add UIO specific operations")
> > Fixes: a5e1018d5e67 ("crypto/caam_jr: add routines to configure HW")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > ---
> > v2:
> >  * Change "job_ring->uio_fd" type suggested by Gagandeep Singh
> > ---
> >  drivers/crypto/caam_jr/caam_jr.c             |  6 ++++--
> >  drivers/crypto/caam_jr/caam_jr_hw_specific.h |  2 +-
> >  drivers/crypto/caam_jr/caam_jr_pvt.h         |  5 +++--
> >  drivers/crypto/caam_jr/caam_jr_uio.c         | 21 ++++++++++++++------
> >  4 files changed, 23 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/crypto/caam_jr/caam_jr.c
> > b/drivers/crypto/caam_jr/caam_jr.c
> > index 5a29dd169..f62d46546 100644
> > --- a/drivers/crypto/caam_jr/caam_jr.c
> > +++ b/drivers/crypto/caam_jr/caam_jr.c
> > @@ -2074,7 +2074,7 @@ static struct rte_security_ops
> > caam_jr_security_ops = {  static void  close_job_ring(struct
> > sec_job_ring_t *job_ring)  {
> > -	if (job_ring->irq_fd) {
> > +	if (job_ring->irq_fd != -1) {
> >  		/* Producer index is frozen. If consumer index is not equal
> >  		 * with producer index, then we have descs to flush.
> >  		 */
> > @@ -2187,7 +2187,7 @@ caam_jr_dev_uninit(struct rte_cryptodev *dev)
> >   *
> >   */
> >  static void *
> > -init_job_ring(void *reg_base_addr, uint32_t irq_id)
> > +init_job_ring(void *reg_base_addr, int irq_id)
> >  {
> >  	struct sec_job_ring_t *job_ring = NULL;
> >  	int i, ret = 0;
> > @@ -2466,6 +2466,8 @@
> > RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv,
> > cryptodev_caam_jr_drv.driver,
> >
> >  RTE_INIT(caam_jr_init_log)
> >  {
> > +	sec_init();
> > +
> Why are you calling a fd initialization function in log registration constructor?
> This is not the right place for any initialization other than logs.
> Also,  sec_init function will not work here, as you are using "g_uio_jr_num"
> whose value will always be 0 during this function call.

Thanks, my bad. I should use 'MAX_SEC_JOB_RINGS'.
The default values of 'g_job_rings' and 'g_uio_job_ring' are 0.
As a result, the validity cannot be checked. what about following:

void sec_uio_job_rings_init(void)
{
	int i;

	for (i = 0; i < MAX_SEC_JOB_RINGS; i++)
		g_uio_job_ring[i].uio_fd = -1;
}

void sec_job_rings_init(void)
{
	int i;

	for (i = 0; i < MAX_SEC_JOB_RINGS; i++)
		g_job_rings[i].irq_fd = -1;
}

RTE_INIT(caam_jr_init)
{
	sec_uio_job_rings_init();
	sec_job_rings_init();
}

> 
> >  	caam_jr_logtype = rte_log_register("pmd.crypto.caam");
> >  	if (caam_jr_logtype >= 0)
> >  		rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE); diff --git
> > a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > index 5f58a585d..ec4539d1b 100644
> > --- a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > +++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > @@ -360,7 +360,7 @@ struct sec_job_ring_t {
> >  				 * bitwise operations.
> >  				 */
> >
> > -	uint32_t irq_fd;	/* The file descriptor used for polling from
> > +	int irq_fd;	/* The file descriptor used for polling from
> >  				 * user space for interrupts notifications
> >  				 */
> >  	uint32_t jr_mode;	/* Model used by SEC Driver to receive
> > diff --git a/drivers/crypto/caam_jr/caam_jr_pvt.h
> > b/drivers/crypto/caam_jr/caam_jr_pvt.h
> > index 98cd4438a..fa9e86ea6 100644
> > --- a/drivers/crypto/caam_jr/caam_jr_pvt.h
> > +++ b/drivers/crypto/caam_jr/caam_jr_pvt.h
> > @@ -216,16 +216,17 @@ calc_chksum(void *buffer, int len)  }  struct
> > uio_job_ring {
> >  	uint32_t jr_id;
> > -	uint32_t uio_fd;
> > +	int uio_fd;
> >  	void *register_base_addr;
> >  	int map_size;
> >  	int uio_minor_number;
> >  };
> >
> >  int sec_cleanup(void);
> > +void sec_init(void);
> >  int sec_configure(void);
> >  struct uio_job_ring *config_job_ring(void); -void
> > free_job_ring(uint32_t uio_fd);
> > +void free_job_ring(int uio_fd);
> >
> >  /* For Dma memory allocation of specified length and alignment */
> > static inline void * diff --git a/drivers/crypto/caam_jr/caam_jr_uio.c
> > b/drivers/crypto/caam_jr/caam_jr_uio.c
> > index b1bb44ca4..133b32084 100644
> > --- a/drivers/crypto/caam_jr/caam_jr_uio.c
> > +++ b/drivers/crypto/caam_jr/caam_jr_uio.c
> > @@ -145,7 +145,7 @@ file_read_first_line(const char root[], const char
> > subdir[],
> >  		 "%s/%s/%s", root, subdir, filename);
> >
> >  	fd = open(absolute_file_name, O_RDONLY);
> > -	SEC_ASSERT(fd > 0, fd, "Error opening file %s",
> > +	SEC_ASSERT(fd >= 0, fd, "Error opening file %s",
> >  			absolute_file_name);
> >
> >  	/* read UIO device name from first line in file */ @@ -322,7 +322,7
> > @@ uio_map_registers(int uio_device_fd, int uio_device_id,  }
> >
> >  void
> > -free_job_ring(uint32_t uio_fd)
> > +free_job_ring(int uio_fd)
> >  {
> >  	struct uio_job_ring *job_ring = NULL;
> >  	int i;
> 
> Please update the check  "if (!uio_fd)".

OK, I will fix it.

> 
> > @@ -347,7 +347,7 @@ free_job_ring(uint32_t uio_fd)
> >  			job_ring->jr_id, job_ring->uio_fd);
> >  	close(job_ring->uio_fd);
> >  	g_uio_jr_num--;
> > -	job_ring->uio_fd = 0;
> > +	job_ring->uio_fd = -1;
> >  	if (job_ring->register_base_addr == NULL)
> >  		return;
> >
> > @@ -370,7 +370,7 @@ uio_job_ring *config_job_ring(void)
> >  	int i;
> >
> >  	for (i = 0; i < MAX_SEC_JOB_RINGS; i++) {
> > -		if (g_uio_job_ring[i].uio_fd == 0) {
> > +		if (g_uio_job_ring[i].uio_fd == -1) {
> >  			job_ring = &g_uio_job_ring[i];
> >  			g_uio_jr_num++;
> >  			break;
> > @@ -389,7 +389,7 @@ uio_job_ring *config_job_ring(void)
> >
> >  	/* Open device file */
> >  	job_ring->uio_fd = open(uio_device_file_name, O_RDWR);
> > -	SEC_ASSERT(job_ring->uio_fd > 0, NULL,
> > +	SEC_ASSERT(job_ring->uio_fd >= 0, NULL,
> >  		"Failed to open UIO device file for job ring %d",
> >  		job_ring->jr_id);
> >
> > @@ -488,12 +488,21 @@ sec_cleanup(void)
> >  		/* I need to close the fd after shutdown UIO commands need to be
> >  		 * sent using the fd
> >  		 */
> > -		if (job_ring->uio_fd != 0) {
> > +		if (job_ring->uio_fd != -1) {
> >  			CAAM_JR_INFO(
> >  			"Closed device file for job ring %d , fd = %d",
> >  			job_ring->jr_id, job_ring->uio_fd);
> >  			close(job_ring->uio_fd);
> > +			job_ring->uio_fd = -1;
> >  		}
> >  	}
> >  	return 0;
> >  }
> > +
> > +void sec_init(void)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < g_uio_jr_num; i++)
> > +		g_uio_job_ring[i].uio_fd = -1;
> > +}
> > --
> > 2.19.1
> >
> 
> There are still some functions(sec_uio_send_command, caam_jr_enable_irqs,
> caam_jr_disable_irqs) in caam_jr_uio.c, which are accepting uio_fd parameter
> as uint32_t, please update them also.
> 
I will update them in next version.

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd

[Gagandeep Singh]

> -----Original Message-----
> From: wangyunjian <wangyunjian@huawei.com>
> Sent: Tuesday, May 12, 2020 4:28 PM
> To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Lilijun (Jerry)
> <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd
> 
> > -----Original Message-----
> > From: Gagandeep Singh [mailto:G.Singh@nxp.com]
> > Sent: Tuesday, May 12, 2020 11:49 AM
> > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Lilijun (Jerry)
> > <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> > stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd
> >
> >
> >
> > > -----Original Message-----
> > > From: wangyunjian <wangyunjian@huawei.com>
> > > Sent: Monday, May 11, 2020 10:07 AM
> > > To: dev@dpdk.org
> > > Cc: Gagandeep Singh <G.Singh@nxp.com>; Hemant Agrawal
> > > <hemant.agrawal@nxp.com>; jerry.lilijun@huawei.com;
> > > xudingke@huawei.com; Yunjian Wang <wangyunjian@huawei.com>;
> > > stable@dpdk.org
> > > Subject: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd
> > >
> > > From: Yunjian Wang <wangyunjian@huawei.com>
> > >
> > > Zero is a valid fd. It will fail to check the fd if the fd is zero.
> > > The "job_ring->uio_fd" is an fd, so define it as "int".
> > >
> > > Fixes: e7a45f3cc245 ("crypto/caam_jr: add UIO specific operations")
> > > Fixes: a5e1018d5e67 ("crypto/caam_jr: add routines to configure HW")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > > ---
> > > v2:
> > >  * Change "job_ring->uio_fd" type suggested by Gagandeep Singh
> > > ---
> > >  drivers/crypto/caam_jr/caam_jr.c             |  6 ++++--
> > >  drivers/crypto/caam_jr/caam_jr_hw_specific.h |  2 +-
> > >  drivers/crypto/caam_jr/caam_jr_pvt.h         |  5 +++--
> > >  drivers/crypto/caam_jr/caam_jr_uio.c         | 21 ++++++++++++++------
> > >  4 files changed, 23 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/drivers/crypto/caam_jr/caam_jr.c
> > > b/drivers/crypto/caam_jr/caam_jr.c
> > > index 5a29dd169..f62d46546 100644
> > > --- a/drivers/crypto/caam_jr/caam_jr.c
> > > +++ b/drivers/crypto/caam_jr/caam_jr.c
> > > @@ -2074,7 +2074,7 @@ static struct rte_security_ops
> > > caam_jr_security_ops = {  static void  close_job_ring(struct
> > > sec_job_ring_t *job_ring)  {
> > > -	if (job_ring->irq_fd) {
> > > +	if (job_ring->irq_fd != -1) {
> > >  		/* Producer index is frozen. If consumer index is not equal
> > >  		 * with producer index, then we have descs to flush.
> > >  		 */
> > > @@ -2187,7 +2187,7 @@ caam_jr_dev_uninit(struct rte_cryptodev *dev)
> > >   *
> > >   */
> > >  static void *
> > > -init_job_ring(void *reg_base_addr, uint32_t irq_id)
> > > +init_job_ring(void *reg_base_addr, int irq_id)
> > >  {
> > >  	struct sec_job_ring_t *job_ring = NULL;
> > >  	int i, ret = 0;
> > > @@ -2466,6 +2466,8 @@
> > > RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv,
> > > cryptodev_caam_jr_drv.driver,
> > >
> > >  RTE_INIT(caam_jr_init_log)
> > >  {
> > > +	sec_init();
> > > +
> > Why are you calling a fd initialization function in log registration constructor?
> > This is not the right place for any initialization other than logs.
> > Also,  sec_init function will not work here, as you are using "g_uio_jr_num"
> > whose value will always be 0 during this function call.
> 
> Thanks, my bad. I should use 'MAX_SEC_JOB_RINGS'.
> The default values of 'g_job_rings' and 'g_uio_job_ring' are 0.
> As a result, the validity cannot be checked. what about following:
> 
> void sec_uio_job_rings_init(void)
> {
> 	int i;
> 
> 	for (i = 0; i < MAX_SEC_JOB_RINGS; i++)
> 		g_uio_job_ring[i].uio_fd = -1;
> }
> 
> void sec_job_rings_init(void)
> {
> 	int i;
> 
> 	for (i = 0; i < MAX_SEC_JOB_RINGS; i++)
> 		g_job_rings[i].irq_fd = -1;
> }
> 
> RTE_INIT(caam_jr_init)
> {
> 	sec_uio_job_rings_init();
> 	sec_job_rings_init();
> }
> 
This looks ok to me. You can combine both init functions into single one. 

> >
> > >  	caam_jr_logtype = rte_log_register("pmd.crypto.caam");
> > >  	if (caam_jr_logtype >= 0)
> > >  		rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE); diff --
> git
> > > a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > index 5f58a585d..ec4539d1b 100644
> > > --- a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > +++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > @@ -360,7 +360,7 @@ struct sec_job_ring_t {
> > >  				 * bitwise operations.
> > >  				 */
> > >
> > > -	uint32_t irq_fd;	/* The file descriptor used for polling from
> > > +	int irq_fd;	/* The file descriptor used for polling from
> > >  				 * user space for interrupts notifications
> > >  				 */
> > >  	uint32_t jr_mode;	/* Model used by SEC Driver to receive
> > > diff --git a/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > b/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > index 98cd4438a..fa9e86ea6 100644
> > > --- a/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > +++ b/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > @@ -216,16 +216,17 @@ calc_chksum(void *buffer, int len)  }  struct
> > > uio_job_ring {
> > >  	uint32_t jr_id;
> > > -	uint32_t uio_fd;
> > > +	int uio_fd;
> > >  	void *register_base_addr;
> > >  	int map_size;
> > >  	int uio_minor_number;
> > >  };
> > >
> > >  int sec_cleanup(void);
> > > +void sec_init(void);
> > >  int sec_configure(void);
> > >  struct uio_job_ring *config_job_ring(void); -void
> > > free_job_ring(uint32_t uio_fd);
> > > +void free_job_ring(int uio_fd);
> > >
> > >  /* For Dma memory allocation of specified length and alignment */
> > > static inline void * diff --git a/drivers/crypto/caam_jr/caam_jr_uio.c
> > > b/drivers/crypto/caam_jr/caam_jr_uio.c
> > > index b1bb44ca4..133b32084 100644
> > > --- a/drivers/crypto/caam_jr/caam_jr_uio.c
> > > +++ b/drivers/crypto/caam_jr/caam_jr_uio.c
> > > @@ -145,7 +145,7 @@ file_read_first_line(const char root[], const char
> > > subdir[],
> > >  		 "%s/%s/%s", root, subdir, filename);
> > >
> > >  	fd = open(absolute_file_name, O_RDONLY);
> > > -	SEC_ASSERT(fd > 0, fd, "Error opening file %s",
> > > +	SEC_ASSERT(fd >= 0, fd, "Error opening file %s",
> > >  			absolute_file_name);
> > >
> > >  	/* read UIO device name from first line in file */ @@ -322,7 +322,7
> > > @@ uio_map_registers(int uio_device_fd, int uio_device_id,  }
> > >
> > >  void
> > > -free_job_ring(uint32_t uio_fd)
> > > +free_job_ring(int uio_fd)
> > >  {
> > >  	struct uio_job_ring *job_ring = NULL;
> > >  	int i;
> >
> > Please update the check  "if (!uio_fd)".
> 
> OK, I will fix it.
> 
> >
> > > @@ -347,7 +347,7 @@ free_job_ring(uint32_t uio_fd)
> > >  			job_ring->jr_id, job_ring->uio_fd);
> > >  	close(job_ring->uio_fd);
> > >  	g_uio_jr_num--;
> > > -	job_ring->uio_fd = 0;
> > > +	job_ring->uio_fd = -1;
> > >  	if (job_ring->register_base_addr == NULL)
> > >  		return;
> > >
> > > @@ -370,7 +370,7 @@ uio_job_ring *config_job_ring(void)
> > >  	int i;
> > >
> > >  	for (i = 0; i < MAX_SEC_JOB_RINGS; i++) {
> > > -		if (g_uio_job_ring[i].uio_fd == 0) {
> > > +		if (g_uio_job_ring[i].uio_fd == -1) {
> > >  			job_ring = &g_uio_job_ring[i];
> > >  			g_uio_jr_num++;
> > >  			break;
> > > @@ -389,7 +389,7 @@ uio_job_ring *config_job_ring(void)
> > >
> > >  	/* Open device file */
> > >  	job_ring->uio_fd = open(uio_device_file_name, O_RDWR);
> > > -	SEC_ASSERT(job_ring->uio_fd > 0, NULL,
> > > +	SEC_ASSERT(job_ring->uio_fd >= 0, NULL,
> > >  		"Failed to open UIO device file for job ring %d",
> > >  		job_ring->jr_id);
> > >
> > > @@ -488,12 +488,21 @@ sec_cleanup(void)
> > >  		/* I need to close the fd after shutdown UIO commands need to
> be
> > >  		 * sent using the fd
> > >  		 */
> > > -		if (job_ring->uio_fd != 0) {
> > > +		if (job_ring->uio_fd != -1) {
> > >  			CAAM_JR_INFO(
> > >  			"Closed device file for job ring %d , fd = %d",
> > >  			job_ring->jr_id, job_ring->uio_fd);
> > >  			close(job_ring->uio_fd);
> > > +			job_ring->uio_fd = -1;
> > >  		}
> > >  	}
> > >  	return 0;
> > >  }
> > > +
> > > +void sec_init(void)
> > > +{
> > > +	int i;
> > > +
> > > +	for (i = 0; i < g_uio_jr_num; i++)
> > > +		g_uio_job_ring[i].uio_fd = -1;
> > > +}
> > > --
> > > 2.19.1
> > >
> >
> > There are still some functions(sec_uio_send_command, caam_jr_enable_irqs,
> > caam_jr_disable_irqs) in caam_jr_uio.c, which are accepting uio_fd parameter
> > as uint32_t, please update them also.
> >
> I will update them in next version.
> 

Re: [dpdk-stable] [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd

[wangyunjian]

> -----Original Message-----
> From: Gagandeep Singh [mailto:G.Singh@nxp.com]
> Sent: Wednesday, May 13, 2020 12:11 PM
> To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Lilijun (Jerry)
> <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> stable@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of fd
> 
> 
> 
> > -----Original Message-----
> > From: wangyunjian <wangyunjian@huawei.com>
> > Sent: Tuesday, May 12, 2020 4:28 PM
> > To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org
> > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Lilijun (Jerry)
> > <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> > stable@dpdk.org
> > Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of
> > fd
> >
> > > -----Original Message-----
> > > From: Gagandeep Singh [mailto:G.Singh@nxp.com]
> > > Sent: Tuesday, May 12, 2020 11:49 AM
> > > To: wangyunjian <wangyunjian@huawei.com>; dev@dpdk.org
> > > Cc: Hemant Agrawal <hemant.agrawal@nxp.com>; Lilijun (Jerry)
> > > <jerry.lilijun@huawei.com>; xudingke <xudingke@huawei.com>;
> > > stable@dpdk.org
> > > Subject: RE: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check
> > > of fd
> > >
> > >
> > >
> > > > -----Original Message-----
> > > > From: wangyunjian <wangyunjian@huawei.com>
> > > > Sent: Monday, May 11, 2020 10:07 AM
> > > > To: dev@dpdk.org
> > > > Cc: Gagandeep Singh <G.Singh@nxp.com>; Hemant Agrawal
> > > > <hemant.agrawal@nxp.com>; jerry.lilijun@huawei.com;
> > > > xudingke@huawei.com; Yunjian Wang <wangyunjian@huawei.com>;
> > > > stable@dpdk.org
> > > > Subject: [dpdk-dev] [PATCH v2] crypto/caam_jr: fix wrong check of
> > > > fd
> > > >
> > > > From: Yunjian Wang <wangyunjian@huawei.com>
> > > >
> > > > Zero is a valid fd. It will fail to check the fd if the fd is zero.
> > > > The "job_ring->uio_fd" is an fd, so define it as "int".
> > > >
> > > > Fixes: e7a45f3cc245 ("crypto/caam_jr: add UIO specific
> > > > operations")
> > > > Fixes: a5e1018d5e67 ("crypto/caam_jr: add routines to configure
> > > > HW")
> > > > Cc: stable@dpdk.org
> > > >
> > > > Signed-off-by: Yunjian Wang <wangyunjian@huawei.com>
> > > > ---
> > > > v2:
> > > >  * Change "job_ring->uio_fd" type suggested by Gagandeep Singh
> > > > ---
> > > >  drivers/crypto/caam_jr/caam_jr.c             |  6 ++++--
> > > >  drivers/crypto/caam_jr/caam_jr_hw_specific.h |  2 +-
> > > >  drivers/crypto/caam_jr/caam_jr_pvt.h         |  5 +++--
> > > >  drivers/crypto/caam_jr/caam_jr_uio.c         | 21
> ++++++++++++++------
> > > >  4 files changed, 23 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/drivers/crypto/caam_jr/caam_jr.c
> > > > b/drivers/crypto/caam_jr/caam_jr.c
> > > > index 5a29dd169..f62d46546 100644
> > > > --- a/drivers/crypto/caam_jr/caam_jr.c
> > > > +++ b/drivers/crypto/caam_jr/caam_jr.c
> > > > @@ -2074,7 +2074,7 @@ static struct rte_security_ops
> > > > caam_jr_security_ops = {  static void  close_job_ring(struct
> > > > sec_job_ring_t *job_ring)  {
> > > > -	if (job_ring->irq_fd) {
> > > > +	if (job_ring->irq_fd != -1) {
> > > >  		/* Producer index is frozen. If consumer index is not equal
> > > >  		 * with producer index, then we have descs to flush.
> > > >  		 */
> > > > @@ -2187,7 +2187,7 @@ caam_jr_dev_uninit(struct rte_cryptodev
> *dev)
> > > >   *
> > > >   */
> > > >  static void *
> > > > -init_job_ring(void *reg_base_addr, uint32_t irq_id)
> > > > +init_job_ring(void *reg_base_addr, int irq_id)
> > > >  {
> > > >  	struct sec_job_ring_t *job_ring = NULL;
> > > >  	int i, ret = 0;
> > > > @@ -2466,6 +2466,8 @@
> > > > RTE_PMD_REGISTER_CRYPTO_DRIVER(caam_jr_crypto_drv,
> > > > cryptodev_caam_jr_drv.driver,
> > > >
> > > >  RTE_INIT(caam_jr_init_log)
> > > >  {
> > > > +	sec_init();
> > > > +
> > > Why are you calling a fd initialization function in log registration
> constructor?
> > > This is not the right place for any initialization other than logs.
> > > Also,  sec_init function will not work here, as you are using
> "g_uio_jr_num"
> > > whose value will always be 0 during this function call.
> >
> > Thanks, my bad. I should use 'MAX_SEC_JOB_RINGS'.
> > The default values of 'g_job_rings' and 'g_uio_job_ring' are 0.
> > As a result, the validity cannot be checked. what about following:
> >
> > void sec_uio_job_rings_init(void)
> > {
> > 	int i;
> >
> > 	for (i = 0; i < MAX_SEC_JOB_RINGS; i++)
> > 		g_uio_job_ring[i].uio_fd = -1;
> > }
> >
> > void sec_job_rings_init(void)
> > {
> > 	int i;
> >
> > 	for (i = 0; i < MAX_SEC_JOB_RINGS; i++)
> > 		g_job_rings[i].irq_fd = -1;
> > }
> >
> > RTE_INIT(caam_jr_init)
> > {
> > 	sec_uio_job_rings_init();
> > 	sec_job_rings_init();
> > }
> >
> This looks ok to me. You can combine both init functions into single one.

OK, the two variables are defined in different files, so cannot be combined. 

Thanks,
Yunjian
> 
> > >
> > > >  	caam_jr_logtype = rte_log_register("pmd.crypto.caam");
> > > >  	if (caam_jr_logtype >= 0)
> > > >  		rte_log_set_level(caam_jr_logtype, RTE_LOG_NOTICE); diff --
> > git
> > > > a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > > b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > > index 5f58a585d..ec4539d1b 100644
> > > > --- a/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > > +++ b/drivers/crypto/caam_jr/caam_jr_hw_specific.h
> > > > @@ -360,7 +360,7 @@ struct sec_job_ring_t {
> > > >  				 * bitwise operations.
> > > >  				 */
> > > >
> > > > -	uint32_t irq_fd;	/* The file descriptor used for polling from
> > > > +	int irq_fd;	/* The file descriptor used for polling from
> > > >  				 * user space for interrupts notifications
> > > >  				 */
> > > >  	uint32_t jr_mode;	/* Model used by SEC Driver to receive
> > > > diff --git a/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > > b/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > > index 98cd4438a..fa9e86ea6 100644
> > > > --- a/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > > +++ b/drivers/crypto/caam_jr/caam_jr_pvt.h
> > > > @@ -216,16 +216,17 @@ calc_chksum(void *buffer, int len)  }
> > > > struct uio_job_ring {
> > > >  	uint32_t jr_id;
> > > > -	uint32_t uio_fd;
> > > > +	int uio_fd;
> > > >  	void *register_base_addr;
> > > >  	int map_size;
> > > >  	int uio_minor_number;
> > > >  };
> > > >
> > > >  int sec_cleanup(void);
> > > > +void sec_init(void);
> > > >  int sec_configure(void);
> > > >  struct uio_job_ring *config_job_ring(void); -void
> > > > free_job_ring(uint32_t uio_fd);
> > > > +void free_job_ring(int uio_fd);
> > > >
> > > >  /* For Dma memory allocation of specified length and alignment */
> > > > static inline void * diff --git
> > > > a/drivers/crypto/caam_jr/caam_jr_uio.c
> > > > b/drivers/crypto/caam_jr/caam_jr_uio.c
> > > > index b1bb44ca4..133b32084 100644
> > > > --- a/drivers/crypto/caam_jr/caam_jr_uio.c
> > > > +++ b/drivers/crypto/caam_jr/caam_jr_uio.c
> > > > @@ -145,7 +145,7 @@ file_read_first_line(const char root[], const
> > > > char subdir[],
> > > >  		 "%s/%s/%s", root, subdir, filename);
> > > >
> > > >  	fd = open(absolute_file_name, O_RDONLY);
> > > > -	SEC_ASSERT(fd > 0, fd, "Error opening file %s",
> > > > +	SEC_ASSERT(fd >= 0, fd, "Error opening file %s",
> > > >  			absolute_file_name);
> > > >
> > > >  	/* read UIO device name from first line in file */ @@ -322,7
> > > > +322,7 @@ uio_map_registers(int uio_device_fd, int uio_device_id,
> > > > }
> > > >
> > > >  void
> > > > -free_job_ring(uint32_t uio_fd)
> > > > +free_job_ring(int uio_fd)
> > > >  {
> > > >  	struct uio_job_ring *job_ring = NULL;
> > > >  	int i;
> > >
> > > Please update the check  "if (!uio_fd)".
> >
> > OK, I will fix it.
> >
> > >
> > > > @@ -347,7 +347,7 @@ free_job_ring(uint32_t uio_fd)
> > > >  			job_ring->jr_id, job_ring->uio_fd);
> > > >  	close(job_ring->uio_fd);
> > > >  	g_uio_jr_num--;
> > > > -	job_ring->uio_fd = 0;
> > > > +	job_ring->uio_fd = -1;
> > > >  	if (job_ring->register_base_addr == NULL)
> > > >  		return;
> > > >
> > > > @@ -370,7 +370,7 @@ uio_job_ring *config_job_ring(void)
> > > >  	int i;
> > > >
> > > >  	for (i = 0; i < MAX_SEC_JOB_RINGS; i++) {
> > > > -		if (g_uio_job_ring[i].uio_fd == 0) {
> > > > +		if (g_uio_job_ring[i].uio_fd == -1) {
> > > >  			job_ring = &g_uio_job_ring[i];
> > > >  			g_uio_jr_num++;
> > > >  			break;
> > > > @@ -389,7 +389,7 @@ uio_job_ring *config_job_ring(void)
> > > >
> > > >  	/* Open device file */
> > > >  	job_ring->uio_fd = open(uio_device_file_name, O_RDWR);
> > > > -	SEC_ASSERT(job_ring->uio_fd > 0, NULL,
> > > > +	SEC_ASSERT(job_ring->uio_fd >= 0, NULL,
> > > >  		"Failed to open UIO device file for job ring %d",
> > > >  		job_ring->jr_id);
> > > >
> > > > @@ -488,12 +488,21 @@ sec_cleanup(void)
> > > >  		/* I need to close the fd after shutdown UIO commands need to
> > be
> > > >  		 * sent using the fd
> > > >  		 */
> > > > -		if (job_ring->uio_fd != 0) {
> > > > +		if (job_ring->uio_fd != -1) {
> > > >  			CAAM_JR_INFO(
> > > >  			"Closed device file for job ring %d , fd = %d",
> > > >  			job_ring->jr_id, job_ring->uio_fd);
> > > >  			close(job_ring->uio_fd);
> > > > +			job_ring->uio_fd = -1;
> > > >  		}
> > > >  	}
> > > >  	return 0;
> > > >  }
> > > > +
> > > > +void sec_init(void)
> > > > +{
> > > > +	int i;
> > > > +
> > > > +	for (i = 0; i < g_uio_jr_num; i++)
> > > > +		g_uio_job_ring[i].uio_fd = -1;
> > > > +}
> > > > --
> > > > 2.19.1
> > > >
> > >
> > > There are still some functions(sec_uio_send_command,
> > > caam_jr_enable_irqs,
> > > caam_jr_disable_irqs) in caam_jr_uio.c, which are accepting uio_fd
> > > parameter as uint32_t, please update them also.
> > >
> > I will update them in next version.
> >