Re: [tegrarcm PATCH v1 1/4] Add option "--pkc"

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 03/04/2016 04:44 PM, Jimmy Zhang wrote:
Add the support code needed to sign the RCM messages with RSA-PSS as
needed to communicate with secured production devices. This mode is
enabled by passing a key via the --pkc command line argument. If such
a key is set the RCM messages will be signed with it as well as the
bootloader.

Signed-off-by: Alban Bedel <alban.bedel@...>

Part of that s-o-b line has been corrupted.

If Alban wrote this, there should be a "From:" line for Alban at the top of the email. Check that "git log" locally shows Alban as the git author of the patch, and "git format-patch" will do the right thing automatically.

Your s-o-b line is missing. It needs to be present even for patches you didn't author, but are simply passing on.


IIUC, this patch allows the user to interact with a chip with PKC enabled, yet without creating a variety of pre-signed binaries/messages. How does that relate to other review comments that complained about having to create pre-signed binaries/messages?

diff --git a/src/main.c b/src/main.c

+	fprintf(stderr, "\t\tSpecify the key file for secured devices. The key should be\n");

s/key/private key/ ?

@@ -175,6 +182,7 @@ int main(int argc, char **argv)
  	int do_read = 0;
  	char *mlfile = NULL;
  	uint32_t mlentry = 0;
+	char *pkc = NULL;

s/pkc/pkc_filename/?

-static int initialize_rcm(uint16_t devid, usb_device_t *usb)
+static int initialize_rcm(uint16_t devid, usb_device_t *usb, const char *keyfile)

s/keyfile/pkc_filename/? (there could be an SBK file instead/too perhaps, and it'd be good to differentiate the two)

A general comment: It'd be good to call this pkc_filename /everywhere/, rather than sometimes pkc, sometimes keyfile, sometimes pkc_keyfile, etc. (One exception might be rsa-pss.c, since that's generic crypto code, not necessarily exclusively used for chip PKC functionality).

diff --git a/src/rsa-pss.cpp b/src/rsa-pss.cpp

+ * Copyright (c) 2015-1016, Avionic Design GmbH

s/1016/2016/. Same comment in the header file.

+extern "C" int rsa_pss_sign(const char *key_file, const unsigned char *msg,
+			int len, unsigned char *sig_buf, unsigned char *modulus_buf)
+{

Here and in rsa_pss_sign_file(), it would be good to validate that the length of the modulus and signature don't exceed the expected size, so that this code doesn't write too much data into sig_buf or modulus_buf.
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [ARM Kernel]     [Linux ARM]     [Linux ARM MSM]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux