Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm_send()

2019-09-17 Thread Jarkko Sakkinen
On Mon, Sep 16, 2019 at 02:04:54PM -0700, Jerry Snitselaar wrote:
> On Mon Sep 16 19, Jerry Snitselaar wrote:
> > On Mon Sep 16 19, Jarkko Sakkinen wrote:
> > > tpm_send() does not give anymore the result back to the caller. This
> > > would require another memcpy(), which kind of tells that the whole
> > > approach is somewhat broken. Instead, as Mimi suggested, this commit
> > > just wraps the data to the tpm_buf, and thus the result will not go to
> > > the garbage.
> > > 
> > > Obviously this assumes from the caller that it passes large enough
> > > buffer, which makes the whole API somewhat broken because it could be
> > > different size than @buflen but since trusted keys is the only module
> > > using this API right now I think that this fix is sufficient for the
> > > moment.
> > > 
> > > In the near future the plan is to replace the parameters with a tpm_buf
> > > created by the caller.
> > > 
> > > Reported-by: Mimi Zohar 
> > > Suggested-by: Mimi Zohar 
> > > Cc: sta...@vger.kernel.org
> > > Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO 
> > > parameter")
> > > Signed-off-by: Jarkko Sakkinen 
> > > ---
> > > drivers/char/tpm/tpm-interface.c | 8 ++--
> > > 1 file changed, 2 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm-interface.c 
> > > b/drivers/char/tpm/tpm-interface.c
> > > index d9ace5480665..2459d36dd8cc 100644
> > > --- a/drivers/char/tpm/tpm-interface.c
> > > +++ b/drivers/char/tpm/tpm-interface.c
> > > @@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, 
> > > size_t buflen)
> > >   if (!chip)
> > >   return -ENODEV;
> > > 
> > > - rc = tpm_buf_init(, 0, 0);
> > > - if (rc)
> > > - goto out;
> > > -
> > > - memcpy(buf.data, cmd, buflen);
> > > + buf.data = cmd;
> > >   rc = tpm_transmit_cmd(chip, , 0, "attempting to a send a command");
> > > - tpm_buf_destroy();
> > > +
> > > out:
> > >   tpm_put_ops(chip);
> > >   return rc;
> > > -- 
> > > 2.20.1
> > > 
> > 
> > Nothing uses the out label any longer so it should be dropped as well, but 
> > other than that...
> > 
> > Acked-by: Jerry Snitselaar 
> 
> sigh (wrong emacs macro hit), that should be:
> 
> Reviewed-by: Jerry Snitselaar 

Thank you! I pushed the commit to master/next.

/Jarkko


Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm_send()

2019-09-16 Thread Jerry Snitselaar

On Mon Sep 16 19, Jerry Snitselaar wrote:

On Mon Sep 16 19, Jarkko Sakkinen wrote:

tpm_send() does not give anymore the result back to the caller. This
would require another memcpy(), which kind of tells that the whole
approach is somewhat broken. Instead, as Mimi suggested, this commit
just wraps the data to the tpm_buf, and thus the result will not go to
the garbage.

Obviously this assumes from the caller that it passes large enough
buffer, which makes the whole API somewhat broken because it could be
different size than @buflen but since trusted keys is the only module
using this API right now I think that this fix is sufficient for the
moment.

In the near future the plan is to replace the parameters with a tpm_buf
created by the caller.

Reported-by: Mimi Zohar 
Suggested-by: Mimi Zohar 
Cc: sta...@vger.kernel.org
Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO parameter")
Signed-off-by: Jarkko Sakkinen 
---
drivers/char/tpm/tpm-interface.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9ace5480665..2459d36dd8cc 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t 
buflen)
if (!chip)
return -ENODEV;

-   rc = tpm_buf_init(, 0, 0);
-   if (rc)
-   goto out;
-
-   memcpy(buf.data, cmd, buflen);
+   buf.data = cmd;
rc = tpm_transmit_cmd(chip, , 0, "attempting to a send a command");
-   tpm_buf_destroy();
+
out:
tpm_put_ops(chip);
return rc;
--
2.20.1



Nothing uses the out label any longer so it should be dropped as well, but 
other than that...

Acked-by: Jerry Snitselaar 


sigh (wrong emacs macro hit), that should be:

Reviewed-by: Jerry Snitselaar 



Re: [PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm_send()

2019-09-16 Thread Jerry Snitselaar

On Mon Sep 16 19, Jarkko Sakkinen wrote:

tpm_send() does not give anymore the result back to the caller. This
would require another memcpy(), which kind of tells that the whole
approach is somewhat broken. Instead, as Mimi suggested, this commit
just wraps the data to the tpm_buf, and thus the result will not go to
the garbage.

Obviously this assumes from the caller that it passes large enough
buffer, which makes the whole API somewhat broken because it could be
different size than @buflen but since trusted keys is the only module
using this API right now I think that this fix is sufficient for the
moment.

In the near future the plan is to replace the parameters with a tpm_buf
created by the caller.

Reported-by: Mimi Zohar 
Suggested-by: Mimi Zohar 
Cc: sta...@vger.kernel.org
Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO parameter")
Signed-off-by: Jarkko Sakkinen 
---
drivers/char/tpm/tpm-interface.c | 8 ++--
1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9ace5480665..2459d36dd8cc 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t 
buflen)
if (!chip)
return -ENODEV;

-   rc = tpm_buf_init(, 0, 0);
-   if (rc)
-   goto out;
-
-   memcpy(buf.data, cmd, buflen);
+   buf.data = cmd;
rc = tpm_transmit_cmd(chip, , 0, "attempting to a send a command");
-   tpm_buf_destroy();
+
out:
tpm_put_ops(chip);
return rc;
--
2.20.1



Nothing uses the out label any longer so it should be dropped as well, but 
other than that...

Acked-by: Jerry Snitselaar 



[PATCH] tpm: Wrap the buffer from the caller to tpm_buf in tpm_send()

2019-09-16 Thread Jarkko Sakkinen
tpm_send() does not give anymore the result back to the caller. This
would require another memcpy(), which kind of tells that the whole
approach is somewhat broken. Instead, as Mimi suggested, this commit
just wraps the data to the tpm_buf, and thus the result will not go to
the garbage.

Obviously this assumes from the caller that it passes large enough
buffer, which makes the whole API somewhat broken because it could be
different size than @buflen but since trusted keys is the only module
using this API right now I think that this fix is sufficient for the
moment.

In the near future the plan is to replace the parameters with a tpm_buf
created by the caller.

Reported-by: Mimi Zohar 
Suggested-by: Mimi Zohar 
Cc: sta...@vger.kernel.org
Fixes: 412eb585587a ("use tpm_buf in tpm_transmit_cmd() as the IO parameter")
Signed-off-by: Jarkko Sakkinen 
---
 drivers/char/tpm/tpm-interface.c | 8 ++--
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d9ace5480665..2459d36dd8cc 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -358,13 +358,9 @@ int tpm_send(struct tpm_chip *chip, void *cmd, size_t 
buflen)
if (!chip)
return -ENODEV;
 
-   rc = tpm_buf_init(, 0, 0);
-   if (rc)
-   goto out;
-
-   memcpy(buf.data, cmd, buflen);
+   buf.data = cmd;
rc = tpm_transmit_cmd(chip, , 0, "attempting to a send a command");
-   tpm_buf_destroy();
+
 out:
tpm_put_ops(chip);
return rc;
-- 
2.20.1