Merge lp:~wlxing/ecryptfs/fix-bug-1678689 into lp:ecryptfs
Status: | Merged |
---|---|
Merged at revision: | 886 |
Proposed branch: | lp:~wlxing/ecryptfs/fix-bug-1678689 |
Merge into: | lp:ecryptfs |
Diff against target: |
12 lines (+2/-0) 1 file modified
src/libecryptfs/cmd_ln_parser.c (+2/-0) |
To merge this branch: | bzr merge lp:~wlxing/ecryptfs/fix-bug-1678689 |
Related bugs: |
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
Tyler Hicks | 2017-05-24 | Approve on 2017-05-25 | |
Michal Hlavinka | 2017-04-03 | Pending | |
Review via email:
|
Description of the change
This revision(r886) is linked to #bug1678689.
Error Case: In the original version, users cannot mount ecryptfs with "-o passphrase_
Error Hint:
http://
Explanation:
Because in the manner of process_comma_tok() function (in the src/libecryptfs
Patch:
Thus, I add two lines in process_comma_tok() to change that case. If we match one "=" character in the string, we don't need to loop and match another "=" again. More details in #change of my branch.
Jason Xing (wlxing) wrote : | # |
Hi Tyler,
Thanks for your check.
Some test cases without "i = st_len":
1) If you add other options like "-o key=passphrase", It would cause
the problem saying "Error attempting to evaluate mount options: [-14]
Bad address...".
2) If you add other options like "-o key=passphrase", It would cause
the problem saying "Error attempting to evaluate mount options: [-22]
Invalid argument...".
Explanation:
If you look at this line "if ((i-j) > 1) {" (it is just several lines
below the break code), you will understand that "i" here means the
length of the whole separate option(eg. key=passpharse). If I just add
break without setting i to st_len, it will skip this line "if ((i-j) >
1) {" and will not allocate any memory for "value".
Does it make sense?
Jason
On Fri, May 26, 2017 at 12:21 AM, Tyler Hicks <email address hidden> wrote:
> Review: Needs Information
>
> Hi Jason - Thanks for the fix!
>
> As you know, the code that you're patching breaks "name=value" pairs up into individual parts. The 'i' variable is used to represent the length of the "name" portion of the string.
>
> I don't think it is correct to set i equal to st_len as st_len is the length of the entire "name=value" string. IIUC, simply adding a break would fix the bug.
>
> What was your reasoning for setting i to st_len?
> --
> https:/
> You are the owner of lp:~wlxing/ecryptfs/fix-bug-1678689.
Jason Xing (wlxing) wrote : | # |
Update
"1) If you add other options like "-o key=passphrase", It would cause
the problem saying "Error attempting to evaluate mount options: [-14]
Bad address..."."
Correct sentence:
"1) If you add other options like "-o passphrase_
the problem saying "Error attempting to evaluate mount options: [-14]
Bad address..."."
Tyler Hicks (tyhicks) wrote : | # |
Thanks for the explanation! You've convinced me that I was wrong about the intent of the "i" variable. I've merged this change.
Hi Jason - Thanks for the fix!
As you know, the code that you're patching breaks "name=value" pairs up into individual parts. The 'i' variable is used to represent the length of the "name" portion of the string.
I don't think it is correct to set i equal to st_len as st_len is the length of the entire "name=value" string. IIUC, simply adding a break would fix the bug.
What was your reasoning for setting i to st_len?