Re: [PATCH] net: don't free uninitialized sockets in dns
ping?
19.08.2015 19:36, Andrei Borzenkov пишет:
18.08.2015 21:03, Josef Bacik пишет:
If we cannot open a connection to our dns server we will have NULL
sockets in
our array, so don't do the cleanup on any sockets that didn't get
created.
We can avoid having holes to start with. Could you test attached patch?
Signed-off-by: Josef Bacik
---
grub-core/net/dns.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
index 9d0c8fc..c356318 100644
--- a/grub-core/net/dns.c
+++ b/grub-core/net/dns.c
@@ -601,7 +601,10 @@ grub_net_dns_lookup (const char *name,
grub_free (data.name);
grub_netbuff_free (nb);
for (j = 0; j < send_servers; j++)
-grub_net_udp_close (sockets[j]);
+{
+ if (sockets[j])
+grub_net_udp_close (sockets[j]);
+}
grub_free (sockets);
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] sparc64: boot performance improvements
Le 10 oct. 2015 3:31 AM, "Eric Snowberg" a
écrit :
>
>
> > On Oct 9, 2015, at 12:50 AM, Andrei Borzenkov
wrote:
> >
> > On Thu, Oct 8, 2015 at 2:20 AM, Eric Snowberg
wrote:
> >>
> >>> On Oct 7, 2015, at 2:36 AM, Andrei Borzenkov
wrote:
> >>>
> >>> On Tue, Oct 6, 2015 at 8:39 PM, Eric Snowberg <
eric.snowb...@oracle.com> wrote:
> Keep of devices open. This can save 6 - 7 seconds per open call and
> can decrease boot times from over 10 minutes to 29 seconds on
> larger SPARC systems. The open/close calls with some vendors'
> SAS controllers cause the entire card to be reinitialized after
> each close.
>
> >>>
> >>> Is it necessary to close these handles before launching kernel?
> >>
> >> On SPARC hardware it is not necessary. I would be interested to hear
if it is necessary on other IEEE1275 platforms.
> >>
> >>> It sounds like it can accumulate quite a lot of them - are there any
> >>> memory limits/restrictions? Also your patch is rather generic and so
> >>> applies to any IEEE1275 platform, I think some of them may have less
> >>> resources. Just trying to understand what run-time cost is.
> >>
> >> The most I have seen are two entries in the linked list. So the
additional memory requirements are very small. I have tried this on a
T2000, T4, and T5.
> >
> > I thought you were speaking about *larger* SPARC servers :)
> >
> > Anyway I'd still prefer if this would be explicitly restricted to
> > Oracle servers. Please look at
> > grub-core/kern/ieee1275/cmain.c:grub_ieee1275_find_options() if new
> > quirk can be added to detect your platform(s).
> >
>
> That makes sense, I’ll restrict it to all sun4v equipment.
>
Not good enough. Some of sun4v probably have the bug I told about in the
other e-mail
> >
> >>
> >> The path name sent into the grub_ieee1275_open function is not
consistent throughout the code, even though it is referencing the same
device. Originally I tried having a global containing the path sent into
grub_ieee1275_open, but this failed due to the various ways of referencing
the same device name. That is why I added the linked list just to be
safe. Caching the grub_ieee1275_ihandle_t this way saves a significant
amount of time, since OF calls are expensive. We were seeing the same
device opened 50+ times in the process of displaying the grub menu and then
launching the kernel.
> >>
> >>>
> Signed-off-by: Eric Snowberg
> ---
> grub-core/kern/ieee1275/ieee1275.c | 39
> 1 files changed, 39 insertions(+), 0 deletions(-)
>
> diff --git a/grub-core/kern/ieee1275/ieee1275.c
b/grub-core/kern/ieee1275/ieee1275.c
> index 9821702..30f973b 100644
> --- a/grub-core/kern/ieee1275/ieee1275.c
> +++ b/grub-core/kern/ieee1275/ieee1275.c
> @@ -19,11 +19,24 @@
>
> #include
> #include
> +#include
> +#include
> +#include
>
> #define IEEE1275_PHANDLE_INVALID ((grub_ieee1275_cell_t) -1)
> #define IEEE1275_IHANDLE_INVALID ((grub_ieee1275_cell_t) 0)
> #define IEEE1275_CELL_INVALID ((grub_ieee1275_cell_t) -1)
>
> +struct grub_of_opened_device
> +{
> + struct grub_of_opened_device *next;
> + struct grub_of_opened_device **prev;
> + grub_ieee1275_ihandle_t ihandle;
> + char *path;
> +};
> +
> +static struct grub_of_opened_device *grub_of_opened_devices;
> +
>
>
> int
> @@ -452,6 +465,18 @@ grub_ieee1275_open (const char *path,
grub_ieee1275_ihandle_t *result)
> }
> args;
>
> + struct grub_of_opened_device *dev;
> +
> + FOR_LIST_ELEMENTS(dev, grub_of_opened_devices)
> +if (grub_strcmp(dev->path, path) == 0)
> + break;
> +
> + if (dev)
> + {
> +*result = dev->ihandle;
> +return 0;
> + }
> +
> INIT_IEEE1275_COMMON (&args.common, "open", 1, 1);
> args.path = (grub_ieee1275_cell_t) path;
>
> @@ -460,6 +485,11 @@ grub_ieee1275_open (const char *path,
grub_ieee1275_ihandle_t *result)
> *result = args.result;
> if (args.result == IEEE1275_IHANDLE_INVALID)
> return -1;
> +
> + dev = grub_zalloc(sizeof(struct grub_of_opened_device));
> >>>
> >>> Error check
> >>
> >> I’ll resubmit V2 with this change
> >>
> >>
> >>
> + dev->path = grub_strdup(path);
> >>>
> >>> Ditto
> >>>
> >>
> >> I’ll resubmit V2 with this change
> >>
> >>
> + dev->ihandle = args.result;
> + grub_list_push(GRUB_AS_LIST_P (&grub_of_opened_devices),
GRUB_AS_LIST (dev));
> return 0;
> }
>
> @@ -473,6 +503,15 @@ grub_ieee1275_close (grub_ieee1275_ihandle_t
ihandle)
> }
> args;
>
> + struct grub_of_opened_device *dev;
> +
> + FOR_LIST_ELEMENTS(dev, grub_of_opened_devices)
> +if (dev->ihandle == ihandle)
> + break;
> +
> + if (dev)
> +return
Re: [PATCH] Avoid NULL pointer dereference in progress module.
21.09.2015 18:11, Dann Frazier пишет:
On Fri, Aug 21, 2015 at 8:24 AM, Dann Frazier
wrote:
On Thu, Aug 20, 2015 at 11:55 AM, Andrei Borzenkov wrote:
18.08.2015 00:57, dann frazier пишет:
grub_net_fs_open() saves off a copy of the file structure it gets passed
and
uses it to create a bufio structure. It then overwrites the passed in file
structure with this new bufio structure. Since file->name doesn't get set
until we return back to grub_file_open(), it means that only the bufio
structure gets a valid file->name. The "real" file's name is left
uninitialized. This leads to a crash when the progress module hook is
called
on it. Fix this by adding a copy of the filename during the switcheroo.
Actually name was already saved off as device->net-name. What about attached
patch? I'll commit leak fix separately.
It does seem like it pokes a hole in the fs abstraction, but it works for me.
hey Andrei,
Did you need anything more from me wrt this fix?
Sorry for delay. We already need to differentiate between disk and net
files in other places anyway. So I think it is OK as a simple fix. I
committed it.
May be bufio needs to be converted to filter but this is too intrusive
for this.
-dann
grub_file_close() will free that string in the success case, we only need
to handle the free in an error. While we're at it, also fix a leaked file
structure in the case that grub_strdup() fails when setting
file->device->net->name.
Finally, make progress more robust by checking for NULL before reading the
name. Andrei noticed that this could occur if grub_strdup() fails in
grub_file_open().
Signed-off-by: dann frazier
---
grub-core/lib/progress.c | 3 +--
grub-core/net/net.c | 14 +-
2 files changed, 14 insertions(+), 3 deletions(-)
diff --git a/grub-core/lib/progress.c b/grub-core/lib/progress.c
index 63a0767..2775554 100644
--- a/grub-core/lib/progress.c
+++ b/grub-core/lib/progress.c
@@ -70,8 +70,7 @@ grub_file_progress_hook_real (grub_disk_addr_t sector
__attribute__ ((unused)),
percent = grub_divmod64 (100 * file->progress_offset,
file->size, 0);
- partial_file_name = grub_strrchr (file->name, '/');
- if (partial_file_name)
+ if (file->name && (partial_file_name = grub_strrchr (file->name,
'/')))
partial_file_name++;
else
partial_file_name = "";
diff --git a/grub-core/net/net.c b/grub-core/net/net.c
index 21a4e94..9f83765 100644
--- a/grub-core/net/net.c
+++ b/grub-core/net/net.c
@@ -1391,7 +1391,17 @@ grub_net_fs_open (struct grub_file *file_out, const
char *name)
file->device->net->packs.last = NULL;
file->device->net->name = grub_strdup (name);
if (!file->device->net->name)
-return grub_errno;
+{
+ grub_free (file);
+ return grub_errno;
+}
+ file->name = grub_strdup (name);
+ if (!file->name)
+{
+ grub_free (file->device->net->name);
+ grub_free (file);
+ return grub_errno;
+}
err = file->device->net->protocol->open (file, name);
if (err)
@@ -1401,6 +1411,7 @@ grub_net_fs_open (struct grub_file *file_out, const
char *name)
grub_netbuff_free (file->device->net->packs.first->nb);
grub_net_remove_packet (file->device->net->packs.first);
}
+ grub_free (file->name);
grub_free (file->device->net->name);
grub_free (file);
return err;
@@ -1414,6 +1425,7 @@ grub_net_fs_open (struct grub_file *file_out, const
char *name)
grub_net_remove_packet (file->device->net->packs.first);
}
file->device->net->protocol->close (file);
+ grub_free (file->name);
grub_free (file->device->net->name);
grub_free (file);
return grub_errno;
___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Make CTRL and ALT keys work as expected on EFI systems (version 5).
26.02.2014 02:12, Peter Jones пишет:
This is version 4.
Changes from version 1:
- handles SHIFT as a modifier
- handles F11 and F12 keys
- uses the handle provided by the system table to find our _EX protocol.
Changes from version 2:
- eliminate duplicate keycode translation.
Changes from version 3:
- Do not add the shift modifier for any ascii character between space
(0x20) and DEL (0x7f); the combination of the modifier and many of the
keys causes it not to be recognized at all. Specifically, if we
include the modifier on any querty punctuation character, i.e.
anything the string "~!@#$%^&*()_+{}|:\"<>?" represents in C, it stops
being recognized whatsoever.
Changes from version 4:
- Always initialize term->data from locate protocol (i.e. make it
unconditional.)
Are there open issues with this patch? Is it used by Fedora? The part
about SHIFT state bothers me, what happens for non-ASCII printable
characters? UEFI spec is extremely vague here.
As currently there is no way to actually input Ctrl-X or similar this is
needed. It may also allow us to actually implement keystatus on EFI.
Signed-off-by: Peter Jones
---
grub-core/term/efi/console.c | 118 +++
include/grub/efi/api.h | 65 +++-
2 files changed, 161 insertions(+), 22 deletions(-)
diff --git a/grub-core/term/efi/console.c b/grub-core/term/efi/console.c
index a37eb84..677eab5 100644
--- a/grub-core/term/efi/console.c
+++ b/grub-core/term/efi/console.c
@@ -104,26 +104,12 @@ const unsigned efi_codes[] =
GRUB_TERM_KEY_DC, GRUB_TERM_KEY_PPAGE, GRUB_TERM_KEY_NPAGE,
GRUB_TERM_KEY_F1,
GRUB_TERM_KEY_F2, GRUB_TERM_KEY_F3, GRUB_TERM_KEY_F4, GRUB_TERM_KEY_F5,
GRUB_TERM_KEY_F6, GRUB_TERM_KEY_F7, GRUB_TERM_KEY_F8, GRUB_TERM_KEY_F9,
-GRUB_TERM_KEY_F10, 0, 0, '\e'
+GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F10, GRUB_TERM_KEY_F11, '\e'
};
-
static int
-grub_console_getkey (struct grub_term_input *term __attribute__ ((unused)))
+grub_efi_translate_key (grub_efi_input_key_t key)
{
- grub_efi_simple_input_interface_t *i;
- grub_efi_input_key_t key;
- grub_efi_status_t status;
-
- if (grub_efi_is_finished)
-return 0;
-
- i = grub_efi_system_table->con_in;
- status = efi_call_2 (i->read_key_stroke, i, &key);
-
- if (status != GRUB_EFI_SUCCESS)
-return GRUB_TERM_NO_KEY;
-
if (key.scan_code == 0)
{
/* Some firmware implementations use VT100-style codes against the spec.
@@ -139,9 +125,98 @@ grub_console_getkey (struct grub_term_input *term
__attribute__ ((unused)))
else if (key.scan_code < ARRAY_SIZE (efi_codes))
return efi_codes[key.scan_code];
+ if (key.unicode_char >= 0x20 && key.unicode_char <= 0x7f)
+return key.unicode_char;
+
return GRUB_TERM_NO_KEY;
}
+static int
+grub_console_getkey_con (struct grub_term_input *term __attribute__ ((unused)))
+{
+ grub_efi_simple_input_interface_t *i;
+ grub_efi_input_key_t key;
+ grub_efi_status_t status;
+
+ i = grub_efi_system_table->con_in;
+ status = efi_call_2 (i->read_key_stroke, i, &key);
+
+ if (status != GRUB_EFI_SUCCESS)
+return GRUB_TERM_NO_KEY;
+
+ return grub_efi_translate_key(key);
+}
+
+static int
+grub_console_getkey_ex(struct grub_term_input *term)
+{
+ grub_efi_key_data_t key_data;
+ grub_efi_status_t status;
+ grub_efi_uint32_t kss;
+ int key = -1;
+
+ grub_efi_simple_text_input_ex_interface_t *text_input = term->data;
+
+ status = efi_call_2 (text_input->read_key_stroke, text_input, &key_data);
+
+ if (status != GRUB_EFI_SUCCESS)
+return GRUB_TERM_NO_KEY;
+
+ kss = key_data.key_state.key_shift_state;
+ key = grub_efi_translate_key(key_data.key);
+
+ if (key == GRUB_TERM_NO_KEY)
+return GRUB_TERM_NO_KEY;
+
+ if (kss & GRUB_EFI_SHIFT_STATE_VALID)
+{
+ if ((kss & GRUB_EFI_LEFT_SHIFT_PRESSED
+ || kss & GRUB_EFI_RIGHT_SHIFT_PRESSED)
+ && !(key >= 0x20 && key <= 0x7f))
+ key |= GRUB_TERM_SHIFT;
+ if (kss & GRUB_EFI_LEFT_ALT_PRESSED || kss & GRUB_EFI_RIGHT_ALT_PRESSED)
+ key |= GRUB_TERM_ALT;
+ if (kss & GRUB_EFI_LEFT_CONTROL_PRESSED
+ || kss & GRUB_EFI_RIGHT_CONTROL_PRESSED)
+ key |= GRUB_TERM_CTRL;
+}
+
+ return key;
+}
+
+static grub_err_t
+grub_efi_console_input_init (struct grub_term_input *term)
+{
+ grub_efi_guid_t text_input_ex_guid =
+GRUB_EFI_SIMPLE_TEXT_INPUT_EX_PROTOCOL_GUID;
+
+ if (grub_efi_is_finished)
+return 0;
+
+ grub_efi_simple_text_input_ex_interface_t *text_input = term->data;
+ if (text_input)
+return 0;
+
+ text_input =
grub_efi_open_protocol(grub_efi_system_table->console_in_handler,
+ &text_input_ex_guid,
+ GRUB_EFI_OPEN_PROTOCOL_GET_PROTOCOL);
+ term->data = (void *)text_input;
+
+ return 0;
+}
+
+static int
+grub_console_getkey (struct grub_term_input *term)
+{
+ if (grub_efi_is_finished)
+return 0;
+
+ if (ter
Re: GNU GRUB maintenance
Am Donnerstag, den 08.10.2015, 21:28 -0400 schrieb Konrad Rzeszutek Wilk: > Also an kernel.org git account could be requested. It is there for > syslinux, dracut, bluetooth, etc. GRUB2 could be hosted there as > well? > What features does kernel.org provide which savannah.gnu.org doestn't have? Plain Web git and space for the tarballs is already available directly from GNU. ___ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel
