Merge lp:~wlxing/ecryptfs/lp1695767 into lp:ecryptfs

[Jason Xing]

Adding just six lines to test whether the value of "num_transitions" is set to zero or not because it stands for how many options user could choose, we can then easily prevent infinite loop and prompt user with error messages. That is to say, if set to zero, that means that eCryptfs goes wrong because there is no selection for user to choose.

[Tyler Hicks (tyhicks)]

Hi Jason - Thanks for the patch!

After reviewing the function that you modified, I think the check for
node->num_transitions being zero can be done much higher. Around line
465, where node->num_transitions is already being checked for being
equal to one, should work fine and then you can drop the call to free
the "prompt" string.

Also, a few comments on patch style...

1) Please put the description of the patch into the commit message
   itself instead of into the pull request description. Someone
   researching why this change was made will typically look into the bzr
   log history but may not look at the Launchpad pull request.

2) The correct syntax for referencing a Launchpad bug is (LP: #1695767).
   The syntax used in the commit message includes an unnecessary "lp".

3) The patch includes a space character between the third and fourth tab
   characters on the line that rc is being assigned.

review: Needs Fixing

[lp:~wlxing/ecryptfs/lp1695767]

888. By Jason Xing on 2017-06-07

Reproduce case:
1) User doesn't install openssl.
2) Run ecryptfs-manager and make selection 3.
It goes in the infinite loop...which means whatever selection you enter next cannot end this command or alter your option.

Explanation for the patch:
Adding several lines to take such a case into consideration, through testing whether "num_transitions" is set to zero or not we could prevent the infinite loop. If it is set to zero, it will return -EINVAL because "num_transitions" means how many options this command could show to users and zero means that user has no option to choose. (LP: #1695767)

[Jason Xing (wlxing)]

Hell Tyler,

Thanks for your instructions!!!

I move those codes much higher and delete "free(prompt);" because it has not allocated memory for "prompt". Also, I delete the space before "rc = ..." line. BTW, I'm wondering how you found the space between the third and the forth tab?

Please review the patch one more time, thanks again.

review: Needs Resubmitting

[Tyler Hicks (tyhicks)]

This looks good. Thanks!

I adjusted the error message and printed it to stderr before committing the fix. Thanks again!

review: Approve

=== modified file 'src/libecryptfs/decision_graph.c'
--- src/libecryptfs/decision_graph.c	2013-10-25 19:45:09 +0000
+++ src/libecryptfs/decision_graph.c	2017-06-07 04:04:13 +0000
@@ -462,6 +462,11 @@
 				       "VALS set\n", __FUNCTION__);
 			memset(&pe_head, 0, sizeof(pe_head));
 			pe = &pe_head;
+			if (!node->num_transitions) {
+				printf("Error attempting to load [%s] module\n", node->mnt_opt_names[0]);
+				rc = -EINVAL;
+				goto out;
+			}
 			if ((node->num_transitions == 1)
 			    && !(node->flags
 				 & ECRYPTFS_PARAM_FORCE_DISPLAY_NODES)) {