| Message ID | 1415172553-5255-1-git-send-email-ivan.hu@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
Ivan, From cking's blog, it replaces "O_NOACCESS 3" with "O_WRONLY | O_RDWR" (=3). However, your are replacing "O_RDONLY 0" with "O_WRONLY | O_RDWR". Is this really intended? Cheers, Alex Hung On 14-11-05 03:29 PM, Ivan Hu wrote: > The efi_runtime driver just doing ioctl() calls, using the flag > (O_WRONLY | O_RDWR) make sure it allows just ioctl() calls and NO accidental > read or writes to the device. > > http://smackerelofopinion.blogspot.co.uk/2012/01/open-using-owronly-ordwr.html > > Signed-off-by: Ivan Hu <ivan.hu@canonical.com> > --- > src/uefi/uefirtmisc/uefirtmisc.c | 2 +- > src/uefi/uefirttime/uefirttime.c | 2 +- > src/uefi/uefirtvariable/uefirtvariable.c | 2 +- > src/uefi/uefivarinfo/uefivarinfo.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c > index bba468e..caafca5 100644 > --- a/src/uefi/uefirtmisc/uefirtmisc.c > +++ b/src/uefi/uefirtmisc/uefirtmisc.c > @@ -53,7 +53,7 @@ static int uefirtmisc_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED; > diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c > index 896e13f..a3125bd 100644 > --- a/src/uefi/uefirttime/uefirttime.c > +++ b/src/uefi/uefirttime/uefirttime.c > @@ -175,7 +175,7 @@ static int uefirttime_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED; > diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c > index f0fd0ce..a19f835 100644 > --- a/src/uefi/uefirtvariable/uefirtvariable.c > +++ b/src/uefi/uefirtvariable/uefirtvariable.c > @@ -99,7 +99,7 @@ static int uefirtvariable_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED; > diff --git a/src/uefi/uefivarinfo/uefivarinfo.c b/src/uefi/uefivarinfo/uefivarinfo.c > index 41296c6..7310931 100644 > --- a/src/uefi/uefivarinfo/uefivarinfo.c > +++ b/src/uefi/uefivarinfo/uefivarinfo.c > @@ -44,7 +44,7 @@ static int uefivarinfo_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED;
On 12/11/14 03:18, Alex Hung wrote: > Ivan, > > From cking's blog, it replaces "O_NOACCESS 3" with "O_WRONLY | O_RDWR" > (=3). However, your are replacing "O_RDONLY 0" with "O_WRONLY | O_RDWR". > Is this really intended? (O_WRONLY | O_RDWR) (also known as O_NOACCESS but not defined in fcntl.h) allows us to perform just the ioctl() and disallows us from accidentally performing reads and writes on the device. Replacing O_RDONLY with (O_WRONLY | O_RDWR) is intentional with this patch set as it stops is from accidentally reading the device and just allows ioctl() access. It is obscure. It is a poorly documented in open() - one has to read the kernel source, but it is a nice little security feature. Colin > > Cheers, > Alex Hung > > On 14-11-05 03:29 PM, Ivan Hu wrote: >> The efi_runtime driver just doing ioctl() calls, using the flag >> (O_WRONLY | O_RDWR) make sure it allows just ioctl() calls and NO >> accidental >> read or writes to the device. >> >> http://smackerelofopinion.blogspot.co.uk/2012/01/open-using-owronly-ordwr.html >> >> >> Signed-off-by: Ivan Hu <ivan.hu@canonical.com> >> --- >> src/uefi/uefirtmisc/uefirtmisc.c | 2 +- >> src/uefi/uefirttime/uefirttime.c | 2 +- >> src/uefi/uefirtvariable/uefirtvariable.c | 2 +- >> src/uefi/uefivarinfo/uefivarinfo.c | 2 +- >> 4 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c >> b/src/uefi/uefirtmisc/uefirtmisc.c >> index bba468e..caafca5 100644 >> --- a/src/uefi/uefirtmisc/uefirtmisc.c >> +++ b/src/uefi/uefirtmisc/uefirtmisc.c >> @@ -53,7 +53,7 @@ static int uefirtmisc_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; >> diff --git a/src/uefi/uefirttime/uefirttime.c >> b/src/uefi/uefirttime/uefirttime.c >> index 896e13f..a3125bd 100644 >> --- a/src/uefi/uefirttime/uefirttime.c >> +++ b/src/uefi/uefirttime/uefirttime.c >> @@ -175,7 +175,7 @@ static int uefirttime_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; >> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c >> b/src/uefi/uefirtvariable/uefirtvariable.c >> index f0fd0ce..a19f835 100644 >> --- a/src/uefi/uefirtvariable/uefirtvariable.c >> +++ b/src/uefi/uefirtvariable/uefirtvariable.c >> @@ -99,7 +99,7 @@ static int uefirtvariable_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; >> diff --git a/src/uefi/uefivarinfo/uefivarinfo.c >> b/src/uefi/uefivarinfo/uefivarinfo.c >> index 41296c6..7310931 100644 >> --- a/src/uefi/uefivarinfo/uefivarinfo.c >> +++ b/src/uefi/uefivarinfo/uefivarinfo.c >> @@ -44,7 +44,7 @@ static int uefivarinfo_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; > >
On 11/05/2014 03:29 PM, Ivan Hu wrote: > The efi_runtime driver just doing ioctl() calls, using the flag > (O_WRONLY | O_RDWR) make sure it allows just ioctl() calls and NO accidental > read or writes to the device. > > http://smackerelofopinion.blogspot.co.uk/2012/01/open-using-owronly-ordwr.html > > Signed-off-by: Ivan Hu <ivan.hu@canonical.com> > --- > src/uefi/uefirtmisc/uefirtmisc.c | 2 +- > src/uefi/uefirttime/uefirttime.c | 2 +- > src/uefi/uefirtvariable/uefirtvariable.c | 2 +- > src/uefi/uefivarinfo/uefivarinfo.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c > index bba468e..caafca5 100644 > --- a/src/uefi/uefirtmisc/uefirtmisc.c > +++ b/src/uefi/uefirtmisc/uefirtmisc.c > @@ -53,7 +53,7 @@ static int uefirtmisc_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED; > diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c > index 896e13f..a3125bd 100644 > --- a/src/uefi/uefirttime/uefirttime.c > +++ b/src/uefi/uefirttime/uefirttime.c > @@ -175,7 +175,7 @@ static int uefirttime_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED; > diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c > index f0fd0ce..a19f835 100644 > --- a/src/uefi/uefirtvariable/uefirtvariable.c > +++ b/src/uefi/uefirtvariable/uefirtvariable.c > @@ -99,7 +99,7 @@ static int uefirtvariable_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED; > diff --git a/src/uefi/uefivarinfo/uefivarinfo.c b/src/uefi/uefivarinfo/uefivarinfo.c > index 41296c6..7310931 100644 > --- a/src/uefi/uefivarinfo/uefivarinfo.c > +++ b/src/uefi/uefivarinfo/uefivarinfo.c > @@ -44,7 +44,7 @@ static int uefivarinfo_init(fwts_framework *fw) > return FWTS_ABORTED; > } > > - fd = open("/dev/efi_runtime", O_RDONLY); > + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); > if (fd == -1) { > fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); > return FWTS_ABORTED; > Acked-by: Alex Hung <alex.hung@canonical.com>
On Thu, Nov 13, 2014 at 2:04 PM, Alex Hung <alex.hung@canonical.com> wrote: > On 11/05/2014 03:29 PM, Ivan Hu wrote: >> The efi_runtime driver just doing ioctl() calls, using the flag >> (O_WRONLY | O_RDWR) make sure it allows just ioctl() calls and NO accidental >> read or writes to the device. >> >> http://smackerelofopinion.blogspot.co.uk/2012/01/open-using-owronly-ordwr.html >> >> Signed-off-by: Ivan Hu <ivan.hu@canonical.com> >> --- >> src/uefi/uefirtmisc/uefirtmisc.c | 2 +- >> src/uefi/uefirttime/uefirttime.c | 2 +- >> src/uefi/uefirtvariable/uefirtvariable.c | 2 +- >> src/uefi/uefivarinfo/uefivarinfo.c | 2 +- >> 4 files changed, 4 insertions(+), 4 deletions(-) >> >> diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c >> index bba468e..caafca5 100644 >> --- a/src/uefi/uefirtmisc/uefirtmisc.c >> +++ b/src/uefi/uefirtmisc/uefirtmisc.c >> @@ -53,7 +53,7 @@ static int uefirtmisc_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; >> diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c >> index 896e13f..a3125bd 100644 >> --- a/src/uefi/uefirttime/uefirttime.c >> +++ b/src/uefi/uefirttime/uefirttime.c >> @@ -175,7 +175,7 @@ static int uefirttime_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; >> diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c >> index f0fd0ce..a19f835 100644 >> --- a/src/uefi/uefirtvariable/uefirtvariable.c >> +++ b/src/uefi/uefirtvariable/uefirtvariable.c >> @@ -99,7 +99,7 @@ static int uefirtvariable_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; >> diff --git a/src/uefi/uefivarinfo/uefivarinfo.c b/src/uefi/uefivarinfo/uefivarinfo.c >> index 41296c6..7310931 100644 >> --- a/src/uefi/uefivarinfo/uefivarinfo.c >> +++ b/src/uefi/uefivarinfo/uefivarinfo.c >> @@ -44,7 +44,7 @@ static int uefivarinfo_init(fwts_framework *fw) >> return FWTS_ABORTED; >> } >> >> - fd = open("/dev/efi_runtime", O_RDONLY); >> + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); >> if (fd == -1) { >> fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); >> return FWTS_ABORTED; >> > > Acked-by: Alex Hung <alex.hung@canonical.com> > Acked-by: Keng-Yu Lin <kengyu@canonical.com>
diff --git a/src/uefi/uefirtmisc/uefirtmisc.c b/src/uefi/uefirtmisc/uefirtmisc.c index bba468e..caafca5 100644 --- a/src/uefi/uefirtmisc/uefirtmisc.c +++ b/src/uefi/uefirtmisc/uefirtmisc.c @@ -53,7 +53,7 @@ static int uefirtmisc_init(fwts_framework *fw) return FWTS_ABORTED; } - fd = open("/dev/efi_runtime", O_RDONLY); + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); if (fd == -1) { fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); return FWTS_ABORTED; diff --git a/src/uefi/uefirttime/uefirttime.c b/src/uefi/uefirttime/uefirttime.c index 896e13f..a3125bd 100644 --- a/src/uefi/uefirttime/uefirttime.c +++ b/src/uefi/uefirttime/uefirttime.c @@ -175,7 +175,7 @@ static int uefirttime_init(fwts_framework *fw) return FWTS_ABORTED; } - fd = open("/dev/efi_runtime", O_RDONLY); + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); if (fd == -1) { fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); return FWTS_ABORTED; diff --git a/src/uefi/uefirtvariable/uefirtvariable.c b/src/uefi/uefirtvariable/uefirtvariable.c index f0fd0ce..a19f835 100644 --- a/src/uefi/uefirtvariable/uefirtvariable.c +++ b/src/uefi/uefirtvariable/uefirtvariable.c @@ -99,7 +99,7 @@ static int uefirtvariable_init(fwts_framework *fw) return FWTS_ABORTED; } - fd = open("/dev/efi_runtime", O_RDONLY); + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); if (fd == -1) { fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); return FWTS_ABORTED; diff --git a/src/uefi/uefivarinfo/uefivarinfo.c b/src/uefi/uefivarinfo/uefivarinfo.c index 41296c6..7310931 100644 --- a/src/uefi/uefivarinfo/uefivarinfo.c +++ b/src/uefi/uefivarinfo/uefivarinfo.c @@ -44,7 +44,7 @@ static int uefivarinfo_init(fwts_framework *fw) return FWTS_ABORTED; } - fd = open("/dev/efi_runtime", O_RDONLY); + fd = open("/dev/efi_runtime", O_WRONLY | O_RDWR); if (fd == -1) { fwts_log_info(fw, "Cannot open efi_runtime driver. Aborted."); return FWTS_ABORTED;
The efi_runtime driver just doing ioctl() calls, using the flag (O_WRONLY | O_RDWR) make sure it allows just ioctl() calls and NO accidental read or writes to the device. http://smackerelofopinion.blogspot.co.uk/2012/01/open-using-owronly-ordwr.html Signed-off-by: Ivan Hu <ivan.hu@canonical.com> --- src/uefi/uefirtmisc/uefirtmisc.c | 2 +- src/uefi/uefirttime/uefirttime.c | 2 +- src/uefi/uefirtvariable/uefirtvariable.c | 2 +- src/uefi/uefivarinfo/uefivarinfo.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-)