Otherwise write_watcher_destroy_notify can be invoked after
GAtMux has been deallocated which results in write after free:
==3952== Invalid write of size 4
==3952== at 0xABF54: write_watcher_destroy_notify (gatmux.c:285)
==3952== by 0x4AF21E7: g_source_callback_unref (gmain.c:1561)
==3952== by 0x4AF2E53: g_source_destroy_internal.constprop.8 (gmain.c:1207)
==3952== by 0x4AF61CF: g_main_dispatch (gmain.c:3177)
==3952== by 0x4AF61CF: g_main_context_dispatch (gmain.c:3769)
==3952== by 0x4AF658F: g_main_loop_run (gmain.c:4034)
==3952== by 0xBDDBB: main (main.c:261)
==3952== Address 0x50c6cb0 is 8 bytes inside a block of size 4,396 free'd
==3952== at 0x4840B28: free (vg_replace_malloc.c:530)
==3952== by 0xACB53: g_at_mux_unref (gatmux.c:642)
==3952== Block was alloc'd at
==3952== at 0x4841BF0: calloc (vg_replace_malloc.c:711)
==3952== by 0xAC9DF: g_at_mux_new (gatmux.c:603)
==3952== by 0xADF2F: g_at_mux_new_gsm0710_basic (gatmux.c:1160)
Leaving them there may result in invalid reads like this:
==2312== Invalid read of size 4
==2312== at 0xAB8C0: dispatch_sources (gatmux.c:134)
==2312== by 0xAC5D3: channel_close (gatmux.c:479)
==2312== by 0x4AE8885: g_io_channel_shutdown (giochannel.c:523)
==2312== by 0x4AE8A1D: g_io_channel_unref (giochannel.c:240)
==2312== by 0xAC423: watch_finalize (gatmux.c:426)
==2312== by 0x4AF2CC9: g_source_unref_internal (gmain.c:2048)
==2312== by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230)
==2312== by 0x4AF44E1: g_source_destroy (gmain.c:1256)
==2312== by 0x4AF5257: g_source_remove (gmain.c:2282)
==2312== by 0xAB5CB: io_shutdown (gatio.c:325)
==2312== by 0xAB667: g_at_io_unref (gatio.c:345)
==2312== by 0xA72C7: at_chat_unref (gatchat.c:972)
==2312== by 0xA829B: g_at_chat_unref (gatchat.c:1446)
==2312== Address 0x51420f0 is 56 bytes inside a block of size 60 free'd
==2312== at 0x4840B28: free (vg_replace_malloc.c:530)
==2312== by 0x4AF2D33: g_source_unref_internal (gmain.c:2075)
==2312== by 0x4AF44E1: g_source_destroy_internal (gmain.c:1230)
==2312== by 0x4AF44E1: g_source_destroy (gmain.c:1256)
==2312== by 0x4AF5257: g_source_remove (gmain.c:2282)
==2312== by 0xAB46B: g_at_io_set_write_handler (gatio.c:283)
==2312== by 0xA713F: at_chat_suspend (gatchat.c:938)
==2312== by 0xA72B7: at_chat_unref (gatchat.c:971)
==2312== by 0xA829B: g_at_chat_unref (gatchat.c:1446)
==2312== Block was alloc'd at
==2312== at 0x4841BF0: calloc (vg_replace_malloc.c:711)
==2312== by 0x4AFB117: g_malloc0 (gmem.c:124)
==2312== by 0x4AF401F: g_source_new (gmain.c:892)
==2312== by 0xAC6A7: channel_create_watch (gatmux.c:506)
==2312== by 0x4AE7C4F: g_io_add_watch_full (giochannel.c:649)
==2312== by 0xAB4EB: g_at_io_set_write_handler (gatio.c:297)
==2312== by 0xA7103: chat_wakeup_writer (gatchat.c:931)
==2312== by 0xA753F: at_chat_send_common (gatchat.c:1045)
==2312== by 0xA850F: g_at_chat_send (gatchat.c:1502)
It's also necessary to add additional references to the sources
for the duration of the dispatch_sources loop because any source
can be removed when any callback is invoked (and not necessarily
the one being dispatched).
Events like +CLCC and +CCWA can have contact name attached to the
end of line. If this field contains odd number of quotation marks,
parser will eventually reject such message as malformatted.
The kernel simply puts a null terminator at index 15 prior to ifr_name
processing. So we do the same.
Original report by:
Sabas Rosales, Blanca E <blanca.e.sabas.rosales@intel.com>
Buffer not null terminated (BUFFER_SIZE_WARNING) buffer_size_warning:
Calling strncpy with a maximum size argument of 16 bytes on destination
array ifr.ifr_ifrn.ifrn_name of size 16 bytes might leave the
destination string unterminated.
67 strncpy(ifr.ifr_name, net->if_name, sizeof(ifr.ifr_name));
CC gatchat/gatchat.o
gatchat/gatchat.c: In function ‘have_line’:
gatchat/gatchat.c:586:28: error: logical not is only applied to the left hand side of comparison [-Werror=logical-not-parentheses]
if (!strncmp(str, "AT", 2) == TRUE)
^
Make the authentication method configurable, CHAP or PAP, defaulting to
CHAP (i.e.: previous behaviour).
Implementation details:
o If PAP is configured, we NAK the CHAP authentication protocol option
in LCP configuration requests and suggest PAP instead. This works
around the amusing requirement of 3GPP TS 29.061 that modems must
send a forced positive acknowledgement of the authentication method
tried (i.e.: the modem will successfully accept any CHAP handshake,
but if the network only supports PAP, the modem will hang up
when it tries and fails to activate the PDP context)
o The PAP Authenticate-Request is resent a hard-coded three times at
ten-second intervals. This may be a bit too persistent. Chances
are if it doesn't work the first time, it'll never work, but the
RFC insists that we MUST retry.
Dundee is not waiting to receive the NO CARRIER notification
to close the IO channel with oFono so that oFono is trying to
send a NO CARRIER although GAtServer is removed.
The current GAtServer implementation had nasty corner cases where
multiple commands were issued on the same command line. The
server_suspend had no effect and we ended up processing the second
command anyway, resulting in interesting side-effects or crashes.
This commit simply discards the rest of the read input if the server
starts processing a command. Since we do not yet support command
abortion we also discard data that arrives when command is being
processed.
Due to the new GAtIO semantics, the receive function is called
immediately if the receive buffer is not empty. This caused certain
funny behavior in non-command (e.g. empty, a/) processing.
Giacinto Cifelli
Marcel Holtmann
Slava Monich
Kuba Pawlak
Denis Kenzior
Antoine Aubert
John Ernberg
Sergey Alirzaev
Philip Paeps
Daniel Wagner
Guillaume Zajac
Oleg Zhurakivskyy