Replace definition of HTTP_PORT with a pre-processor macro that converts
the constant to the correct grub_uint16_t type.
Change "port" local variable definition in http_establish() to have the
same type.
Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com
There are 3 implementations of a GUID in GRUB. Replace them with
a common one, placed in types.h.
It uses the "packed" flavor of the GUID structs, the alignment attribute
is dropped, since it is not required.
Signed-off-by: Oliver Steffen <osteffen@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Now that GCC can generate function calls using the correct calling
convention for us, we can stop using the efi_call_XX() wrappers, and
just dereference the function pointers directly.
This avoids the untyped variadic wrapper routines, which means better
type checking for the method calls.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When trying to resolve DNS names into IP addresses, the DNS code fails
from time to time with the following error:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
error: ../../grub-core/net/dns.c:688:no DNS record found.
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
This happens when both IPv4 and IPv6 queries are performed against the
DNS server (e.g. 8.8.8.8) but there is no IP returned for IPv6 query, as
shown below:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
grub> net_del_dns 192.168.122.1
grub> net_add_dns 8.8.8.8
grub> net_nslookup ipv4.test-ipv6.com
error: ../../grub-core/net/dns.c:688:no DNS record found.
grub> net_nslookup ipv4.test-ipv6.com
216.218.228.115
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
The root cause is the code exiting prematurely when the data->addresses
buffer has been allocated in recv_hook(), even if there was no address
returned last time recv_hook() executed.
Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When deleting the DNS server, we get the following error message:
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
grub> net_del_dns 192.168.122.1
error: ../../grub-core/net/dns.c:646:no DNS reply received.
-------- 8< ---------------- 8< ---------------- 8< ---------------- 8< --------
This happens because the implementation is broken, it does a "add"
internally instead of a "delete".
Signed-off-by: Renaud Métrich <rmetrich@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Allow specifying port numbers for http and tftp paths and allow IPv6
addresses to be recognized with brackets around them, which is required
to specify a port number.
Co-authored-by: Aaron Miller <aaronmiller@fb.com>
Signed-off-by: Aaron Miller <aaronmiller@fb.com>
Co-authored-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The notation introduced in ac8a37dda (net/http: Allow use of non-standard
TCP/IP ports) contradicts that used in downstream distributions including
Fedora, RHEL, Debian, Ubuntu, and others. Revert it and apply the downstream
notation which was originally proposed to the GRUB in 2016.
This reverts commit ac8a37dda (net/http: Allow use of non-standard TCP/IP ports).
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the function send_dhcp_packet(), added an error check for the return
value of grub_netbuff_push().
Fixes: CID 404614
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
When GRUB runs on top of EFI firmware, it only has access to block and
network device abstractions exposed by the firmware, and it is up to the
firmware to quiesce the underlying hardware when exiting boot services
and handing over to the OS.
This is especially important for network devices, to prevent incoming
packets from being DMA'd straight into memory after the OS has taken
over but before it has managed to reconfigure the network hardware.
GRUB handles this by means of the grub_net_fini_hw() preboot hook, which
is executed before calling into the booted image. This means that all
network devices disappear or become inoperable before the EFI stub
executes on EFI targeted builds. This is problematic as it prevents the
EFI stub from calling back into GRUB provided protocols such as
LoadFile2 for the initrd, which we will provide in a subsequent patch.
So add a flag that indicates to the network core that EFI network
devices should not be closed when grub_net_fini_hw() is called.
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
Reviewed-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The netmask configured in firmware is not respected on ppc64 (big endian).
When 255.255.252.0 is set as netmask in firmware, the following is the
value of bootpath string in grub_ieee1275_parse_bootpath():
/vdevice/l-lan@30000002:speed=auto,duplex=auto,192.168.88.10,,192.168.89.113,192.168.88.1,5,5,255.255.252.0,512
The netmask in this bootpath is not a problem, since it's a value specified
in firmware. But the value of subnet_mask.ipv4 was set with 0xfffffc00, and
__builtin_ctz(~grub_le_to_cpu32(subnet_mask.ipv4)) returned 16 (not 22).
As a result, 16 was used for netmask wrongly:
1111 1111 1111 1111 1111 1100 0000 0000 # subnet_mask.ipv4(=0xfffffc00)
0000 0000 1111 1100 1111 1111 1111 1111 # grub_le_to_cpu32(subnet_mask.ipv4)
1111 1111 0000 0011 0000 0000 0000 0000 # ~grub_le_to_cpu32(subnet_mask.ipv4)
and the count of zero with __builtin_ctz() can be 16. This patch changes
it as below:
1111 1111 1111 1111 1111 1100 0000 0000 # subnet_mask.ipv4(=0xfffffc00)
0000 0000 1111 1100 1111 1111 1111 1111 # grub_le_to_cpu32(subnet_mask.ipv4)
1111 1111 1111 1111 1111 1100 0000 0000 # grub_be_to_cpu32(subnet_mask.ipv4)
0000 0000 0000 0000 0000 0011 1111 1111 # ~grub_be_to_cpu32(subnet_mask.ipv4)
The count of zero with __builtin_clz() can be 22 (clz counts the number
of one bits preceding the most significant zero bit).
Signed-off-by: Masahiro Matsuya <mmatsuya@redhat.com>
Signed-off-by: Robbie Harwood <rharwood@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
It will be desirable in the future to allow having the read hook modify the
data passed back from a read function call on a disk or file. This adds that
infrastructure and has no impact on code flow for existing uses of the read
hook. Also changed is that now when the read hook callback is called it can
also indicate what error code should be sent back to the read caller.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Patrick Steinhardt <ps@pks.im>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In a similar vein to the previous patch, parse_line() would write
a NUL byte past the end of the buffer if there was an HTTP header
with a LF rather than a CRLF.
RFC-2616 says:
Many HTTP/1.1 header field values consist of words separated by LWS
or special characters. These special characters MUST be in a quoted
string to be used within a parameter value (as defined in section 3.6).
We don't support quoted sections or continuation lines, etc.
If we see an LF that's not part of a CRLF, bail out.
Fixes: CVE-2022-28734
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
GRUB has special code for handling an http header that is split
across two packets.
The code tracks the end of line by looking for a "\n" byte. The
code for split headers has always advanced the pointer just past the
end of the line, whereas the code that handles unsplit headers does
not advance the pointer. This extra advance causes the length to be
one greater, which breaks an assumption in parse_line(), leading to
it writing a NUL byte one byte past the end of the buffer where we
reconstruct the line from the two packets.
It's conceivable that an attacker controlled set of packets could
cause this to zero out the first byte of the "next" pointer of the
grub_mm_region structure following the current_line buffer.
Do not advance the pointer in the split header case.
Fixes: CVE-2022-28734
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
It's possible for data->sock to get torn down in tcp error handling.
If we unconditionally tear it down again we will end up doing writes
to an offset of the NULL pointer when we go to tear it down again.
Detect if it has been torn down and don't do it again.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Under tftp errors, we print a tftp error message from the tftp header.
However, the tftph pointer is a pointer inside nb, the netbuff. Previously,
we were freeing the nb and then dereferencing it. Don't do that, use it
and then free it later.
This isn't really _bad_ per se, especially as we're single-threaded, but
it trips up fuzzers.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A malicious tftp server can cause UAFs and a double free.
An attempt to read from a network file is handled by grub_net_fs_read(). If
the read is at an offset other than the current offset, grub_net_seek_real()
is invoked.
In grub_net_seek_real(), if a backwards seek cannot be satisfied from the
currently received packets, and the underlying transport does not provide
a seek method, then grub_net_seek_real() will close and reopen the network
protocol layer.
For tftp, the ->close() call goes to tftp_close() and frees the tftp_data_t
file->data. The file->data pointer is not nulled out after the free.
If the ->open() call fails, the file->data will not be reallocated and will
continue point to a freed memory block. This could happen from a server
refusing to send the requisite ack to the new tftp request, for example.
The seek and the read will then fail, but the grub_file continues to exist:
the failed seek does not necessarily cause the entire file to be thrown
away (e.g. where the file is checked to see if it is gzipped/lzio/xz/etc.,
a read failure is interpreted as a decompressor passing on the file, not as
an invalidation of the entire grub_file_t structure).
This means subsequent attempts to read or seek the file will use the old
file->data after free. Eventually, the file will be close()d again and
file->data will be freed again.
Mark a net_fs file that doesn't reopen as broken. Do not permit read() or
close() on a broken file (seek is not exposed directly to the file API -
it is only called as part of read, so this blocks seeks as well).
As an additional defence, null out the ->data pointer if tftp_open() fails.
That would have lead to a simple null pointer dereference rather than
a mess of UAFs.
This may affect other protocols, I haven't checked.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
I don't really understand what's going on here but fuzzing found
a bug where we read past the end of check_with. That's a C string,
so use grub_strlen() to make sure we don't overread it.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
grub_net_dns_lookup() takes as inputs a pointer to an array of addresses
("addresses") for the given name, and pointer to a number of addresses
("naddresses"). grub_net_dns_lookup() is responsible for allocating
"addresses", and the caller is responsible for freeing it if
"naddresses" > 0.
The DNS recv_hook will sometimes set and free the addresses array,
for example if the packet is too short:
if (ptr + 10 >= nb->tail)
{
if (!*data->naddresses)
grub_free (*data->addresses);
grub_netbuff_free (nb);
return GRUB_ERR_NONE;
}
Later on the nslookup command code unconditionally frees the "addresses"
array. Normally this is fine: the array is either populated with valid
data or is NULL. But in these sorts of error cases it is neither NULL
nor valid and we get a double-free.
Only free "addresses" if "naddresses" > 0.
It looks like the other use of grub_net_dns_lookup() is not affected.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
A netbuff shouldn't be too huge. It's bounded by MTU and TCP segment
reassembly. If we are asked to create one that is unreasonably big, refuse.
This is a hardening measure: if we hit this code, there's a bug somewhere
else that we should catch and fix.
This commit:
- stops the bug propagating any further.
- provides a spot to instrument in e.g. fuzzing to try to catch these bugs.
I have put instrumentation (e.g. __builtin_trap() to force a crash) here and
have not been able to find any more crashes.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
We can receive packets with invalid IP fragmentation information. This
can lead to rsm->total_len underflowing and becoming very large.
Then, in grub_netbuff_alloc(), we add to this very large number, which can
cause it to overflow and wrap back around to a small positive number.
The allocation then succeeds, but the resulting buffer is too small and
subsequent operations can write past the end of the buffer.
Catch the underflow here.
Fixes: CVE-2022-28733
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The commit 848724273e4 (net/net: Avoid unnecessary calls to
grub_net_tcp_retransmit()) needs to have its condition inverted to avoid
unnecessary calls to grub_net_tcp_retransmit(). As it is, it creates many
unnecessary calls and does not call grub_net_tcp_retransmit() when needed.
The call to grub_net_tcp_retransmit() should only be made when
grub_net_cards does _not_ equal NULL, meaning that there are potentially
network cards that need TCP retransmission.
Fixes: 848724273e4 (net/net: Avoid unnecessary calls to grub_net_tcp_retransmit())
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
During UEFI PXE boot in IPv6 network, if the DHCP server adopts stateful
automatic configuration, then the client receives a ICMP6_ROUTER_ADVERTISE
multicast message from the server. This may be received without the interface
having a configured network address, so orig_inf will be NULL, which can lead
to a NULL dereference when creating the default route. Actually, in this case,
the client obtains the default route through DHCPv6 instead of RA messages.
So if orig_inf == NULL and route_inf == NULL, we should not set the
default route.
Fixes: https://savannah.gnu.org/bugs/?62072
Signed-off-by: Qiumiao Zhang <zhangqiumiao1@huawei.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The GRUB is failing to build with GCC-12 in many places like this:
In function 'init_cbfsdisk',
inlined from 'grub_mod_init' at ../../grub-core/fs/cbfs.c:391:3:
../../grub-core/fs/cbfs.c:345:7: error: array subscript 0 is outside array bounds of 'grub_uint32_t[0]' {aka 'unsigned int[]'} [-Werror=array-bounds]
345 | ptr = *(grub_uint32_t *) 0xfffffffc;
| ~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
This is caused by GCC regression in 11/12 [1]. In a nut shell, the
warning is about detected invalid accesses at non-zero offsets to NULL
pointers. Since hardwired constant address is treated as NULL plus an
offset in the same underlying code, the warning is therefore triggered.
Instead of inserting #pragma all over the places where literal pointers
are accessed to avoid diagnosing array-bounds, we can try to borrow the
idea from Linux kernel that the absolute_pointer() macro [2][3] is used
to disconnect a pointer using literal address from it's original object,
hence GCC won't be able to make assumptions on the boundary while doing
pointer arithmetic. With that we can greatly reduce the code we have to
cover up by making initial literal pointer assignment to use the new
wrapper but not having to track everywhere literal pointers are
accessed. This also makes code looks cleaner.
Please note the grub_absolute_pointer() macro requires to be invoked in
a function as long as it is compound expression. Some global variables
with literal pointers has been changed to local ones in order to use
grub_absolute_pointer() to initialize it. The shuffling is basically done
in a selective and careful way that the variable's scope doesn't matter
being local or global, for example, the global variable must not get
modified at run time throughout. For the record, here's the list of
global variables got shuffled in this patch:
grub-core/commands/i386/pc/drivemap.c:int13slot
grub-core/term/i386/pc/console.c:bios_data_area
grub-core/term/ns8250.c:serial_hw_io_addr
[1] https://gcc.gnu.org/bugzilla/show_bug.cgi?id=99578
[2] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler.h#L180
[3] https://elixir.bootlin.com/linux/v5.16.14/source/include/linux/compiler-gcc.h#L31
Signed-off-by: Michael Chang <mchang@suse.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This patch handles automatic configuration of VLAN when booting from PXE
on UEFI hardware.
Signed-off-by: Chad Kimes <chkimes@github.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Previously there was no way to set the 802.1Q VLAN identifier, despite
support for vlantag in the net module. The only location vlantag was
being populated was from PXE boot and only for Open Firmware hardware.
This commit allows users to manually configure VLAN information for any
interface.
Example usage:
grub> net_ls_addr
efinet1 00:11:22:33:44:55 192.0.2.100
grub> net_set_vlan efinet1 100
grub> net_ls_addr
efinet1 00:11:22:33:44:55 192.0.2.100 vlan100
grub> net_set_vlan efinet1 0
efinet1 00:11:22:33:44:55 192.0.2.100
Signed-off-by: Chad Kimes <chkimes@github.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the function grub_net_ipv6_get_link_local(), grub_net_network_level_address_t
addr is called but isn't being initialized. This results in the member
grub_dns_option_t option being filled with junk data from the stack. We can
prevent this by setting the option member in addr to 0.
Fixes: CID 375033
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the function grub_net_configure_by_dhcp_ack(),
grub_net_network_level_address_t addr is called but isn't being initialized.
This results in the member grub_dns_option_t option being filled with junk data
from the stack. To prevent this, we can set the option member in addr to 0.
Fixes: CID 375036
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the function grub_net_arp_receive(), grub_net_network_level_address_t
sender_addr and target_addr are being called but aren't being initialized.
In both of these structs, each member is being set to a value except for
grub_dns_option_t option. This results in this member being filled with junk
data from the stack. To prevent this, we can set the option member in both
structs to 0.
Fixes: CID 375030
Signed-off-by: Alec Brown <alec.r.brown@oracle.com>
Reviewed-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
If the machine has network cards found, but there are no tcp open sockets
(because the user doesn't use the network to boot), then grub_net_tcp_retransmit()
should be a noop. Thus GRUB doesn't need to call grub_get_time_ms(), which
does a call into firmware on powerpc-ieee1275, and probably other targets.
So only call grub_get_time_ms() if there are tcp sockets.
Aside from improving performance, its also useful to stay out of the firmware
as much as possible when debugging via QEMU because its a pain to get back
in to GRUB execution. grub_net_tcp_retransmit() can get called very frequently
via grub_net_poll_cards_idle() when GRUB is waiting for a keypress
(grub_getkey_noblock() calls grub_net_poll_cards_idle()). This can be annoying
when debugging an issue in GRUB on PowerPC in QEMU with GDB when GRUB is waiting
for a keypress because interrupting via GDB nearly always lands in the OpenBIOS
firmware's milliseconds call.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In grub_net_poll_cards_idle_real(), only call grub_net_tcp_retransmit() if there
are network cards found. If there are no network card found, there can be no
tcp sockets to transmit on. So no need to go through that logic.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This looks like it was a copy/paste error. If the net module is unloaded,
grub_net_poll_cards_idle should be NULL so that GRUB does not try to call
a function which now doesn't exist.
Signed-off-by: Glenn Washburn <development@efficientek.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
GRUB netbuff structure members track 2 different things: the extent of memory
allocated for the packet, and the extent of memory currently being worked on.
This works out in the structure as follows:
nb->head: beginning of the allocation
nb->data: beginning of the working data
nb->tail: end of the working data
nb->end: end of the allocation
The head and end pointers are set in grub_netbuff_alloc() and do not change.
The data and tail pointers are initialised to point at start of the
allocation (that is, head == data == tail initially), and are then
manipulated by grub_netbuff_*() functions. Key functions are as follows:
- grub_netbuff_put(): "put" more data into the packet - advance nb->tail
- grub_netbuff_unput(): trim the tail of the packet - retract nb->tail
- grub_netbuff_pull(): "consume" some packet data - advance nb->data
- grub_netbuff_reserve(): reserve space for future headers - advance nb->data and nb->tail
- grub_netbuff_push(): "un-consume" data to allow headers to be written - retract nb->data
Each of those functions does some form of error checking. For example,
grub_netbuff_put() does not allow nb->tail to exceed nb->end, and
grub_netbuff_push() does not allow nb->data to be before nb->head.
However, grub_netbuff_pull()'s error checking is a bit weird. It advances nb->data
and checks that it does not exceed nb->end. That allows you to get into the
situation where nb->data > nb->tail, which should not be.
Make grub_netbuff_pull() check against both nb->tail and nb->end. In theory just
checking against ->tail should be sufficient but the extra check should be
cheap and seems like good defensive practice.
Signed-off-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
VLAN configuration seems to have never worked on little-endian systems.
This is likely because VLANTAG_IDENTIFIER is not byte-swapped before
copying into the net buffer, nor is inf->vlantag. We can resolve this by
using grub_cpu_to_be16{_compile_time}() and its inverse when copying
VLAN info to/from the net buffer.
Signed-off-by: Chad Kimes <chkimes@github.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Allow the use of HTTP servers listening on ports other 80. This is done
with an extension to the http notation:
(http[,server[,port]])
- or -
(http[,server[:port]])
Signed-off-by: Stephen Balousek <sbalousek@wickedloop.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Create a library function for CloseProtocol() and use it for the SNP driver.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
In the context of the implementation of the EFI_LOAD_FILE2_PROTOCOL for
the initial ramdisk it was observed that opening the SNP protocol failed.
https://lists.gnu.org/archive/html/grub-devel/2021-10/msg00020.html
This is due to an incorrect call to CloseProtocol().
The first parameter of CloseProtocol() is the handle, not the interface.
We call OpenProtocol() with ControllerHandle == NULL. Hence we must also
call CloseProtcol() with ControllerHandel == NULL.
Each call of OpenProtocol() for the same network card handle is expected to
return the same interface pointer. If we want to close the protocol which
we opened non-exclusively when searching for a card, we have to do this
before opening the protocol exclusively.
As there is no guarantee that we successfully open the protocol add checks
in the transmit and receive functions.
Reported-by: Andreas Schwab <schwab@linux-m68k.org>
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Some filesystems nowadays use 64-bit types for timestamps. So, update
grub_dirhook_info struct to use an grub_int64_t type to store mtime.
This also updates the grub_unixtime2datetime() function to receive
a 64-bit timestamp argument and do 64-bit-safe divisions.
All the remaining conversion from 32-bit to 64-bit should be safe, as
32-bit to 64-bit attributions will be implicitly casted. The most
critical part in the 32-bit to 64-bit conversion is in the function
grub_unixtime2datetime() where it needs to deal with the 64-bit type.
So, for that, the grub_divmod64() helper has been used.
These changes enables the GRUB to support dates beyond y2038.
Signed-off-by: Carlos Maiolino <cmaiolino@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
The static code analysis tool, Parfait, reported that the valid of
file->data was left referencing memory that was freed by the call to
grub_free(data) where data was initialized from file->data.
To ensure that there is no unintentional access to this memory
referenced by file->data we should set the pointer to NULL.
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
It is always possible that grub_zalloc() could fail, so we should check for
a NULL return. Otherwise we run the risk of dereferencing a NULL pointer.
Fixes: CID 296221
Signed-off-by: Darren Kenny <darren.kenny@oracle.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Commit 781b3e5efc3 (tftp: Do not use priority queue) caused a regression
when fetching files over TFTP whose size is bigger than 65535 * block size.
grub> linux /images/pxeboot/vmlinuz
grub> echo $?
0
grub> initrd /images/pxeboot/initrd.img
error: timeout reading '/images/pxeboot/initrd.img'.
grub> echo $?
28
It is caused by the block number counter being a 16-bit field, which leads
to a maximum file size of ((1 << 16) - 1) * block size. Because GRUB sets
the block size to 1024 octets (by using the TFTP Blocksize Option from RFC
2348 [0]), the maximum file size that can be transferred is 67107840 bytes.
The TFTP PROTOCOL (REVISION 2) RFC 1350 [1] does not mention what a client
should do when a file size is bigger than the maximum, but most TFTP hosts
support the block number counter to be rolled over. That is, acking a data
packet with a block number of 0 is taken as if the 65356th block was acked.
It was working before because the block counter roll-over was happening due
an overflow. But that got fixed by the mentioned commit, which led to the
regression when attempting to fetch files larger than the maximum size.
To allow TFTP file transfers of unlimited size again, re-introduce a block
counter roll-over so the data packets are acked preventing the timeouts.
[0]: https://tools.ietf.org/html/rfc2348
[1]: https://tools.ietf.org/html/rfc1350
Fixes: 781b3e5efc3 (tftp: Do not use priority queue)
Suggested-by: Peter Jones <pjones@redhat.com>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
There is not need to reassemble the order of blocks. Per RFC 1350,
server must wait for the ACK, before sending next block. Data packets
can be served immediately without putting them to priority queue.
Logic to handle incoming packet is this:
- if packet block id equal to expected block id, then
process the packet,
- if packet block id is less than expected - this is retransmit
of old packet, then ACK it and drop the packet,
- if packet block id is more than expected - that shouldn't
happen, just drop the packet.
It makes the tftp receive path code simpler, smaller and faster.
As a benefit, this change fixes CID# 73624 and CID# 96690, caused
by following while loop:
while (cmp_block (grub_be_to_cpu16 (tftph->u.data.block), data->block + 1) == 0)
where tftph pointer is not moving from one iteration to another, causing
to serve same packet again. Luckily, double serving didn't happen due to
data->block++ during the first iteration.
Fixes: CID 73624, CID 96690
Signed-off-by: Alexey Makhalov <amakhalov@vmware.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This attempts to fix the places where we do the following where
arithmetic_expr may include unvalidated data:
X = grub_malloc(arithmetic_expr);
It accomplishes this by doing the arithmetic ahead of time using grub_add(),
grub_sub(), grub_mul() and testing for overflow before proceeding.
Among other issues, this fixes:
- allocation of integer overflow in grub_video_bitmap_create()
reported by Chris Coulson,
- allocation of integer overflow in grub_png_decode_image_header()
reported by Chris Coulson,
- allocation of integer overflow in grub_squash_read_symlink()
reported by Chris Coulson,
- allocation of integer overflow in grub_ext2_read_symlink()
reported by Chris Coulson,
- allocation of integer overflow in read_section_as_string()
reported by Chris Coulson.
Fixes: CVE-2020-14309, CVE-2020-14310, CVE-2020-14311
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
This modifies most of the places we do some form of:
X = malloc(Y * Z);
to use calloc(Y, Z) instead.
Among other issues, this fixes:
- allocation of integer overflow in grub_png_decode_image_header()
reported by Chris Coulson,
- allocation of integer overflow in luks_recover_key()
reported by Chris Coulson,
- allocation of integer overflow in grub_lvm_detect()
reported by Chris Coulson.
Fixes: CVE-2020-14308
Signed-off-by: Peter Jones <pjones@redhat.com>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
Nested functions are not supported in C, but are permitted as an extension
in the GNU C dialect. Commit cb2f15c5448 ("normal/main: Search for specific
config files for netboot") added a nested function which caused the build
to break when compiling with clang.
Break that out into a static helper function to make the code portable again.
Reported-by: Daniel Axtens <dja@axtens.net>
Signed-off-by: Javier Martinez Canillas <javierm@redhat.com>
Tested-by: Daniel Axtens <dja@axtens.net>
Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
